[PATCH net-next] net/sched: don't use dynamic lockdep keys with clsact/ingress/noqueue

[PATCH net-next] net/sched: don't use dynamic lockdep keys with clsact/ingress/noqueue

[Davide Caratti]

Currently we are registering one dynamic lockdep key for each allocated
qdisc, to avoid false deadlock reports when mirred (or TC eBPF) redirects
packets to another device while the root lock is acquired [1].
Since dynamic keys are a limited resource, we can save them at least for
qdiscs that are not meant to acquire the root lock in the traffic path, or
to carry traffic at all. A good (initial) example is:

 - clsact
 - ingress
 - noqueue

Don't register dynamic keys for the above schedulers, so that we hit
MAX_LOCKDEP_KEYS later in our tests.

[1] https://github.com/multipath-tcp/mptcp_net-next/issues/451

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 include/net/pkt_sched.h | 22 ++++++++++++++++++++++
 net/sched/sch_api.c     |  2 +-
 net/sched/sch_generic.c |  8 +++-----
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index e703c507d0da..e68a7bbb84b4 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -308,4 +308,26 @@ static inline unsigned int qdisc_peek_len(struct Qdisc *sch)
 	return len;
 }
 
+static inline void qdisc_lock_init(struct Qdisc *sch, const struct Qdisc_ops *ops)
+{
+	bool skip_dynamic_key = (ops->static_flags & TCQ_F_INGRESS) ||
+				(ops == &noqueue_qdisc_ops);
+
+	if (!skip_dynamic_key)
+		lockdep_register_key(&sch->root_lock_key);
+
+	spin_lock_init(&sch->q.lock);
+	if (!skip_dynamic_key)
+		lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
+}
+
+static inline void qdisc_lock_uninit(struct Qdisc *sch, const struct Qdisc_ops *ops)
+{
+	bool skip_dynamic_key = (ops->static_flags & TCQ_F_INGRESS) ||
+				(ops == &noqueue_qdisc_ops);
+
+	if (!skip_dynamic_key)
+		lockdep_unregister_key(&sch->root_lock_key);
+}
+
 #endif
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f56b18c8aebf..443c116e8663 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1353,7 +1353,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 		ops->destroy(sch);
 	qdisc_put_stab(rtnl_dereference(sch->stab));
 err_out3:
-	lockdep_unregister_key(&sch->root_lock_key);
+	qdisc_lock_uninit(sch, ops);
 	netdev_put(dev, &sch->dev_tracker);
 	qdisc_free(sch);
 err_out2:
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 852e603c1755..98ffe64de51f 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -955,9 +955,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 	__skb_queue_head_init(&sch->gso_skb);
 	__skb_queue_head_init(&sch->skb_bad_txq);
 	gnet_stats_basic_sync_init(&sch->bstats);
-	lockdep_register_key(&sch->root_lock_key);
-	spin_lock_init(&sch->q.lock);
-	lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
+	qdisc_lock_init(sch, ops);
 
 	if (ops->static_flags & TCQ_F_CPUSTATS) {
 		sch->cpu_bstats =
@@ -987,7 +985,7 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 
 	return sch;
 errout1:
-	lockdep_unregister_key(&sch->root_lock_key);
+	qdisc_lock_uninit(sch, ops);
 	kfree(sch);
 errout:
 	return ERR_PTR(err);
@@ -1076,7 +1074,7 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
 	if (ops->destroy)
 		ops->destroy(qdisc);
 
-	lockdep_unregister_key(&qdisc->root_lock_key);
+	qdisc_lock_uninit(qdisc, ops);
 	bpf_module_put(ops, ops->owner);
 	netdev_put(dev, &qdisc->dev_tracker);
 
-- 
2.52.0

Re: [PATCH net-next] net/sched: don't use dynamic lockdep keys with clsact/ingress/noqueue

[Jakub Kicinski]

On Thu, 29 Jan 2026 16:24:35 +0100 Davide Caratti wrote:
> +static inline void qdisc_lock_init(struct Qdisc *sch, const struct Qdisc_ops *ops)

wrap at 80 please

> +{
> +	bool skip_dynamic_key = (ops->static_flags & TCQ_F_INGRESS) ||
> +				(ops == &noqueue_qdisc_ops);
> +
> +	if (!skip_dynamic_key)
> +		lockdep_register_key(&sch->root_lock_key);
> +
> +	spin_lock_init(&sch->q.lock);
> +	if (!skip_dynamic_key)
> +		lockdep_set_class(&sch->q.lock, &sch->root_lock_key);

is there a reason for the order of things here? The code flow would be
far more natural with:

{
	spin_lock_init(&sch->q.lock);

	/* Skip dynamic keys for qdiscs which can't nest */
	if (ops->static_flags & TCQ_F_INGRESS ||
            ops == &noqueue_qdisc_ops)
		return;

	lockdep_register_key(&sch->root_lock_key);
	lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
}

> +}
> +
> +static inline void qdisc_lock_uninit(struct Qdisc *sch, const struct Qdisc_ops *ops)
> +{
> +	bool skip_dynamic_key = (ops->static_flags & TCQ_F_INGRESS) ||
> +				(ops == &noqueue_qdisc_ops);
> +
> +	if (!skip_dynamic_key)
> +		lockdep_unregister_key(&sch->root_lock_key);
-- 
pw-bot: cr

Re: [PATCH net-next] net/sched: don't use dynamic lockdep keys with clsact/ingress/noqueue

[Davide Caratti]

On Fri, Jan 30, 2026 at 07:29:21PM -0800, Jakub Kicinski wrote:
> On Thu, 29 Jan 2026 16:24:35 +0100 Davide Caratti wrote:
> > +static inline void qdisc_lock_init(struct Qdisc *sch, const struct Qdisc_ops *ops)
> 
> wrap at 80 please

hi Jakub, thanks for reading. Sure, will fix in v2.
 
> > +{
> > +	bool skip_dynamic_key = (ops->static_flags & TCQ_F_INGRESS) ||
> > +				(ops == &noqueue_qdisc_ops);
> > +
> > +	if (!skip_dynamic_key)
> > +		lockdep_register_key(&sch->root_lock_key);
> > +
> > +	spin_lock_init(&sch->q.lock);
> > +	if (!skip_dynamic_key)
> > +		lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
> 
> is there a reason for the order of things here? 

I was self-convinced that CONFIG_DEBUG_SPINLOCK and similar would
complain with the ordering below, but I'm now seeing empirically
that it doesn't.

> The code flow would be far more natural with:
> 
> {
> 	spin_lock_init(&sch->q.lock);
> 
> 	/* Skip dynamic keys for qdiscs which can't nest */
> 	if (ops->static_flags & TCQ_F_INGRESS ||
>             ops == &noqueue_qdisc_ops)
> 		return;
> 
> 	lockdep_register_key(&sch->root_lock_key);
> 	lockdep_set_class(&sch->q.lock, &sch->root_lock_key);
> }

will do some additional tests and re-send this way if it doesn't
complain.

-- 
davide