netdev
[Top] [All Lists]

Re: [PATCH] PKT_SCHED: Initialize list field in dummy qdiscs

To: Thomas Graf <tgraf@xxxxxxx>
Subject: Re: [PATCH] PKT_SCHED: Initialize list field in dummy qdiscs
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Sun, 07 Nov 2004 18:02:52 +0100
Cc: davem@xxxxxxxxxxxxx, netdev@xxxxxxxxxxx, spam@xxxxxxxxxxxxx, kuznet@xxxxxxxxxxxxx, jmorris@xxxxxxxxxx
In-reply-to: <20041107163330.GB31969@xxxxxxxxxxxxxx>
References: <20041105175812.GZ12289@xxxxxxxxxxxxxx> <418BC40E.8080402@xxxxxxxxx> <20041105194303.GA12289@xxxxxxxxxxxxxx> <20041106011843.GI12289@xxxxxxxxxxxxxx> <418C2D40.9020300@xxxxxxxxx> <20041106015931.GA28715@xxxxxxxxxxxxxx> <20041106145036.GB28715@xxxxxxxxxxxxxx> <418DE37E.2050504@xxxxxxxxx> <20041107140015.GA31969@xxxxxxxxxxxxxx> <418E4B2E.1070407@xxxxxxxxx> <20041107163330.GB31969@xxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.3) Gecko/20041008 Debian/1.7.3-5
Thomas Graf wrote:

I might have misunderstood you in this point, so you increment a
refcnt in qdisc_lookup and decrement in once you're done with
the reference? I thought you wanted to to bh spin locks. I'm not
sure how you want to do this without creating races.

Example:

We get a RTM_DELQDISC request so you'll increment refcnt in
qdisc_lookup and decrement it right before you call qdisc_destroy
so it actually can be deleted. The rcu callback works fine
and will set up a another rcu callback for the destroying of
the inner qdiscs. Right at this time you get a RTM_GETQDISC for
that inner qdisc so you'll lock on it, then the rcu callback
comes in and cannot delete the inner qdisc anymore. Do you want
to sleep in softirq context?

This can't happen. Once the inner qdiscs refcnt drops to zero
they are removed from the list, before the rcu callback is scheduled.
Once the RCU callback is scheduled it can't be found anymore.

BTW: An alternative, quite unintrusive solution is to prevent anyone
from finding the inner qdiscs after the outer one has been destroyed.
This can be done be keeping inner qdiscs on qdisc->qdisc_list and only
keep the top-level qdisc in struct net_device. Of course, this makes
walking all qdiscs more complicated. A generation counter for the
top-level qdisc should also work.

Regards
Patrick


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