netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarek Poplawski <jarkao2@o2.pl>
To: Dave Jones <davej@redhat.com>
Cc: Patrick McHardy <kaber@trash.net>,
	hadi@cyberus.ca, netdev@vger.kernel.org, davem@davemloft.net
Subject: Re: tc related lockdep warning.
Date: Wed, 27 Sep 2006 10:54:27 +0200	[thread overview]
Message-ID: <20060927085427.GA1619@ff.dom.local> (raw)
In-Reply-To: <20060926212034.GA3134@redhat.com>

On Tue, Sep 26, 2006 at 05:20:34PM -0400, Dave Jones wrote:
> On Tue, Sep 26, 2006 at 06:15:21PM +0200, Patrick McHardy wrote:
>  > Patrick McHardy wrote:
>  > > jamal wrote:
>  > > 
>  > >>Yes, that looks plausible. Can you try making those changes and see if
>  > >>the warning is gone?
>  > > 
>  > >
>  > > I think this points to a bigger brokeness caused by the move of
>  > > dev->qdisc to RCU. It means destruction of filters and actions doesn't
>  > > necessarily happens in user-context and thus not protected by the rtnl
>  > > anymore.
>  > 
>  > I looked into this and we indeed still have lots of problems from that
>  > broken RCU patch. Basically all locking (qdiscs, classifiers, actions,
>  > estimators) assumes that updates are only done in process context and
>  > thus read_lock doesn't need bottem half protection. Quite a few things
>  > also assume that updates only happen under the RTNL and don't need
>  > any further protection if not used during packet processing.
>  > 
>  > Instead of "fixing" all this I suggest something like this (untested)
>  > patch instead. Since only the dev->qdisc pointer is protected by RCU,
>  > but enqueue and the qdisc tree are still protected by dev->qdisc_lock,
>  > we can perform destruction of the tree immediately and only do the
>  > final free in the rcu callback, as long as we make sure not to enqueue
>  > anything to a half-way destroyed qdisc.
> 
> With this patch, I get no lockdep warnings, but the machine locks up completely.
> I hooked up a serial console, and found this..
...

Sorry for my not humble and simplistic opinion, but I'd dare
to remind you are changing "stable" version and even without
this lockups this patch would look very "serious". Why don't
try to restore not-rcu version of qdisc_destroy which looks
not lot to do.

Jarek P.

  reply	other threads:[~2006-09-27  8:50 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-24 21:29 tc related lockdep warning Dave Jones
2006-09-25 12:43 ` Jarek Poplawski
2006-09-25 12:47   ` jamal
2006-09-25 13:05     ` Jarek Poplawski
2006-09-25 13:29     ` Patrick McHardy
2006-09-26 16:15       ` Patrick McHardy
2006-09-26 21:20         ` Dave Jones
2006-09-27  8:54           ` Jarek Poplawski [this message]
2006-09-27  9:57             ` Patrick McHardy
2006-09-28 12:17             ` Patrick McHardy
2006-09-28 13:13               ` Jarek Poplawski
2006-09-28 14:20                 ` Stephen Hemminger
2006-09-29  6:28                   ` Jarek Poplawski
2006-09-27 10:14           ` Patrick McHardy
2006-09-27 14:41             ` Ismail Donmez
2006-09-27 12:07           ` Patrick McHardy
2006-09-27 17:26             ` Ismail Donmez
2006-09-27 17:33             ` Dave Jones
2006-09-27 23:53             ` David Miller
2006-09-28  9:07               ` Jarek Poplawski
2006-09-28  8:17             ` Jarek Poplawski

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=20060927085427.GA1619@ff.dom.local \
    --to=jarkao2@o2.pl \
    --cc=davej@redhat.com \
    --cc=davem@davemloft.net \
    --cc=hadi@cyberus.ca \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    /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).