Re: [PATCH net-next 0/2] net/sched: don't wait for RCU when destroying qdisc lockdep keys

[Boqun Feng <boqun@kernel.org>]

On Mon, Oct 19, 2026 at 10:02:23AM -0700, Vlad Poenaru wrote:
> From: Vlad Poenaru <thevlad@meta.com>
> 
> On kernels with CONFIG_PROVE_LOCKING, every qdisc except ingress, clsact
> and noqueue has a dynamic lockdep key, and destroying the qdisc calls
> lockdep_unregister_key(), which waits for an expedited RCU grace period.
> Replacing an mq root destroys one qdisc per TX queue under RTNL, so a
> single "tc qdisc replace dev eth0 root mq" waits for that many grace
> periods. Commit 7a3cedafccf8 ("lockdep: Speed up lockdep_unregister_key()
> with expedited RCU synchronization") made each wait shorter as a
> temporary workaround until hazard pointers are available.
> 
> The wait is redundant for qdiscs: the qdisc is freed with call_rcu()
> after its key is unregistered, so the key's memory already outlives an
> RCU grace period. Patch 1 revives Bart Van Assche's
> lockdep_unregister_key_nosync() [1], with the grace-period requirement
> spelled out, and patch 2 uses it for qdisc keys, as Eric suggested [2].
> 
> Boqun was concerned that a nosync variant is hard to enforce and review,
> since a later refactor could move the free [3]. For qdiscs the RCU-deferred
> free is not optional: datapath readers find the qdisc under RCU, so
> freeing it synchronously would already be a use-after-free regardless of
> lockdep. Patch 2 also documents the requirement at qdisc_lock_uninit().

I think my concern is still valid: for lockdep PoV,
lockdep_unregister_key_nosync() is an API that users have to use it
carefully, because it leaves the unregistration process in a stage that
you need to guarantee that the lockdep key reclamation is delayed by an
RCU grace period. Or in other words, do you have a way to prevent a user
from doing lockdep_unregister_key_nosync() but kfree(key), or some sort
of debugging to find a mis-use?

Regards,
Boqun

> The hazptr-based replacement for the wait is still under discussion [4];
> if it lands, the nosync variant can stay as the no-wait path or be
> dropped.
> 
> On an x86_64 KASAN + PROVE_LOCKING kernel (64 vCPU KVM guest, 64-queue
> netdevsim device, 214K lock chains) this removes 5.3 ms of grace-period
> wait from each of the 65 qdisc destroys in "tc qdisc replace dev eth0
> root handle 1: mq".
> 
> A separate lockdep patch, sent to tip, makes the zap_class() part of
> lockdep_unregister_key() cheaper [5]; the two are independent. Nilay
> Shroff's pending "lockdep: Fix subclass resource leak in
> lockdep_unregister_key()" [6] touches the same function as patch 1;
> whichever lands second needs a trivial rebase.
> 
> Peter, Boqun, Waiman: if patch 1 looks fine, an Ack for taking it
> through net-next with its user would be appreciated.
> 
> [1] https://lore.kernel.org/all/20250203175939.3133477-2-bvanassche@acm.org/
> [2] https://lore.kernel.org/all/CANn89iLvu_ijSV+FnrZ6qei_Ty2c9V-RjwLMk24+oOS91bNjaQ@mail.gmail.com/
> [3] https://lore.kernel.org/all/Z-OPya5HoqbKmMGj@Mac.home/
> [4] https://lore.kernel.org/all/20261005171529.1378809-9-kunwu.chan@gmail.com/
> [5] https://lore.kernel.org/all/20261019170018.699298-1-vlad.wing@gmail.com/
> [6] https://lore.kernel.org/all/20260727182312.3750145-1-nilay@linux.ibm.com/
> 
> Bart Van Assche (1):
>   lockdep: Introduce lockdep_unregister_key_nosync()
> 
> Vlad Poenaru (1):
>   net/sched: Don't wait for RCU when unregistering the qdisc lockdep key
> 
>  include/linux/lockdep.h  |  5 +++++
>  include/net/pkt_sched.h  |  3 ++-
>  kernel/locking/lockdep.c | 36 +++++++++++++++++++++++++-----------
>  net/sched/sch_generic.c  |  2 +-
>  4 files changed, 33 insertions(+), 13 deletions(-)
> 
> -- 
> 2.53.0-Meta
>