All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlad Buslov <vladbu@mellanox.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Buslov <vladbu@mellanox.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"jhs@mojatatu.com" <jhs@mojatatu.com>,
	"xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>,
	"jiri@resnulli.us" <jiri@resnulli.us>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock
Date: Fri, 20 Sep 2019 06:34:28 +0000	[thread overview]
Message-ID: <vbfsgorzdam.fsf@mellanox.com> (raw)
In-Reply-To: <66e68933-a553-e078-b92b-6f629c740328@gmail.com>


On Fri 20 Sep 2019 at 01:49, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 9/19/19 1:14 PM, Vlad Buslov wrote:
>> Recent changes that removed rtnl dependency from rules update path of tc
>> also made tcf_block_put() function sleeping. This function is called from
>> ops->destroy() of several Qdisc implementations, which in turn is called by
>> qdisc_put(). Some Qdiscs call qdisc_put() while holding sch tree spinlock,
>> which results sleeping-while-atomic BUG.
>>
>
>
> Note that calling qdisc_put() while holding sch tree lock can also
> trigger deadlocks.
>
> For example sch_pie.c has a del_timer_sync() in pie_destroy(),
> while the pie_timer() timer handler acquires the root_lock.
>
> (there are other cases like that, SFQ for example)

These and other examples of sleeping calls in cls APIs used by
tcf_block_put() that I described in one of my previous emails make me
think that putting might_sleep() at the beginning of qdisc_put() would
be a good idea, instead of waiting for syzbot to find correct
combination to trigger a crash.

  reply	other threads:[~2019-09-20  6:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-19 20:14 [PATCH net v2 0/3] Fix Qdisc destroy issues caused by adding fine-grained locking to filter API Vlad Buslov
2019-09-19 20:14 ` [PATCH net v2 1/3] net: sched: sch_htb: don't call qdisc_put() while holding tree lock Vlad Buslov
2019-09-19 22:49   ` Eric Dumazet
2019-09-20  6:34     ` Vlad Buslov [this message]
2019-09-20  1:05   ` Cong Wang
2019-09-20  6:27     ` Vlad Buslov
2019-09-20 16:53       ` Cong Wang
2019-09-19 20:14 ` [PATCH net v2 2/3] net: sched: multiq: " Vlad Buslov
2019-09-19 20:14 ` [PATCH net v2 3/3] net: sched: sch_sfb: " Vlad Buslov

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=vbfsgorzdam.fsf@mellanox.com \
    --to=vladbu@mellanox.com \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=xiyou.wangcong@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.