From: Eric Dumazet <edumazet@google.com>
To: Davide Caratti <dcaratti@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Cong Wang <xiyou.wangcong@gmail.com>,
Jiri Pirko <jiri@resnulli.us>,
netdev@vger.kernel.org, xmu@redhat.com, cpaasch@apple.com
Subject: Re: [PATCH net-next] net/sched: fix false lockdep warning on qdisc root lock
Date: Wed, 6 Dec 2023 11:16:39 +0100 [thread overview]
Message-ID: <CANn89iLXjstLx-=hFR0Rhav462+9pH3JTyE45t+nyiszKKCPTQ@mail.gmail.com> (raw)
In-Reply-To: <73065927a49619fcd60e5b765c929f899a66cd1a.1701853200.git.dcaratti@redhat.com>
On Wed, Dec 6, 2023 at 10:04 AM Davide Caratti <dcaratti@redhat.com> wrote:
>
> Xiumei and Cristoph reported the following lockdep splat, it complains of
> the qdisc root being taken twice:
>
> ============================================
> WARNING: possible recursive locking detected
> 6.7.0-rc3+ #598 Not tainted
> --------------------------------------------
> swapper/2/0 is trying to acquire lock:
> ffff888177190110 (&sch->q.lock){+.-.}-{2:2}, at: __dev_queue_xmit+0x1560/0x2e70
>
> but task is already holding lock:
> ffff88811995a110 (&sch->q.lock){+.-.}-{2:2}, at: __dev_queue_xmit+0x1560/0x2e70
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&sch->q.lock);
> lock(&sch->q.lock);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 5 locks held by swapper/2/0:
> #0: ffff888135a09d98 ((&in_dev->mr_ifc_timer)){+.-.}-{0:0}, at: call_timer_fn+0x11a/0x510
> #1: ffffffffaaee5260 (rcu_read_lock){....}-{1:2}, at: ip_finish_output2+0x2c0/0x1ed0
> #2: ffffffffaaee5200 (rcu_read_lock_bh){....}-{1:2}, at: __dev_queue_xmit+0x209/0x2e70
> #3: ffff88811995a110 (&sch->q.lock){+.-.}-{2:2}, at: __dev_queue_xmit+0x1560/0x2e70
> #4: ffffffffaaee5200 (rcu_read_lock_bh){....}-{1:2}, at: __dev_queue_xmit+0x209/0x2e70
>
>
Can you add a Fixes: tag ?
Also, what is the interaction with htb_set_lockdep_class_child(), have
you tried to use HTB after your patch ?
Could htb_set_lockdep_class_child() be removed ?
> CC: Xiumei Mu <xmu@redhat.com>
> Reported-by: Cristoph Paasch <cpaasch@apple.com>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/451
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> include/net/sch_generic.h | 1 +
> net/sched/sch_generic.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index dcb9160e6467..a395ca76066c 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -126,6 +126,7 @@ struct Qdisc {
>
> struct rcu_head rcu;
> netdevice_tracker dev_tracker;
> + struct lock_class_key root_lock_key;
> /* private data */
> long privdata[] ____cacheline_aligned;
> };
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 8dd0e5925342..da3e1ea42852 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -944,7 +944,9 @@ 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);
>
> if (ops->static_flags & TCQ_F_CPUSTATS) {
> sch->cpu_bstats =
> @@ -1064,6 +1066,7 @@ static void __qdisc_destroy(struct Qdisc *qdisc)
> if (ops->destroy)
> ops->destroy(qdisc);
>
> + lockdep_unregister_key(&qdisc->root_lock_key);
> module_put(ops->owner);
> netdev_put(qdisc_dev(qdisc), &qdisc->dev_tracker);
>
> --
> 2.41.0
>
next prev parent reply other threads:[~2023-12-06 10:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-06 9:03 [PATCH net-next] net/sched: fix false lockdep warning on qdisc root lock Davide Caratti
2023-12-06 10:16 ` Eric Dumazet [this message]
2023-12-06 10:25 ` Eric Dumazet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANn89iLXjstLx-=hFR0Rhav462+9pH3JTyE45t+nyiszKKCPTQ@mail.gmail.com' \
--to=edumazet@google.com \
--cc=cpaasch@apple.com \
--cc=davem@davemloft.net \
--cc=dcaratti@redhat.com \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=xiyou.wangcong@gmail.com \
--cc=xmu@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).