netdev
[Top] [All Lists]

Re: ACPI/HT or Packet Scheduler BUG?

To: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Subject: Re: ACPI/HT or Packet Scheduler BUG?
From: Thomas Graf <tgraf@xxxxxxx>
Date: Sat, 16 Apr 2005 13:06:39 +0200
Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>, hadi@xxxxxxxxxx, netdev <netdev@xxxxxxxxxxx>, Tarhon-Onu Victor <mituc@xxxxxxxxxxxxxx>, kuznet@xxxxxxxxxxxxx, devik@xxxxxx, linux-kernel@xxxxxxxxxxxxxxx, Patrick McHardy <kaber@xxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>
In-reply-to: <20050416014906.GA3291@xxxxxxxxxxxxxxxxxxx>
References: <Pine.LNX.4.61.0504081225510.27991@xxxxxxxxxxxxxxxxxxxxxxxx> <Pine.LNX.4.61.0504121526550.4822@xxxxxxxxxxxxxxxxxxxxxxxx> <Pine.LNX.4.61.0504141840420.13546@xxxxxxxxxxxxxxxxxxxxxxxx> <1113601029.4294.80.camel@xxxxxxxxxxxxxxxxxxxxx> <1113601446.17859.36.camel@xxxxxxxxxxxxxxxxxxxxx> <1113602052.4294.89.camel@xxxxxxxxxxxxxxxxxxxxx> <20050415225422.GF4114@xxxxxxxxxxxxxx> <20050416014906.GA3291@xxxxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
* Herbert Xu <20050416014906.GA3291@xxxxxxxxxxxxxxxxxxx> 2005-04-16 11:49
> On Fri, Apr 15, 2005 at 10:54:22PM +0000, Thomas Graf wrote:
> >
> > Another case were it's not locked is upon a deletion of a class where
> > the class deletes its inner qdisc, although there is only one case
> > how this can happen and that's under rtnl semaphore so there is no
> > way we can have a dumper at the same time.
> 
> Sorry, that's where tc went astray :)
> 
> The assumption that the rtnl is held during dumping is false.  It is
> only true for the initial dump call.  All subsequent dumps are not
> protected by the rtnl.

Ahh.. it's the unlocked subsequent dump calls that are _still_ running
when the destroy is invoked. That's where Patrick and I went wrong
when we tried to fix this issue. We set our t0 to qdisc_destroy and
didn't really consider any prior unlocked tasks still running.

> In fact the whole qdisc locking is a mess.  It's a cross-breed
> between locking with ad-hoc reference counting and RCU.  What's
> more, the RCU is completely useless too because for the case
> where we actually have a queue we still end up taking the spin
> lock on each transmit! I think someone's been benchmarking the
> loopback device again :)

It's not completely useless, it speeds up the deletion classful
qdiscs having some depth. However, it's not worth the locking
troubles I guess.

> Here is a quick'n'dirty fix to the problem at hand.

I think it's pretty clean but it doesn't solve the problem completely,
see below.

> This patch tries to ensure that all top-level calls to qdisc_destroy
> come under the tree lock.  As Thomas correctedly pointed out, most
> of the other qdisc_destroy calls occur after the top qdisc has been
> unlinked from the device qdisc_list.  However, someone should go
> through each one of the remaining ones (they're all in the individual
> sch_* implementations) and make sure that this assumption is really
> true.

qdisc_destroy can still be invoked without qdisc_tree_lock via the
deletion of a class when it calls qdisc_destroy to destroy its
leaf qdisc.

> If anyone has cycles to spare and a stomach strong enough for
> this stuff, here is your chance :)

I will look into this.

<Prev in Thread] Current Thread [Next in Thread>