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

[Patrick McHardy <kaber@trash.net>]

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