On Thu, Aug 21, 2008 at 11:55:22AM +0000, Jarek Poplawski wrote:

Sure, here is a scenario:

cpu1					cpu2
rtnl_lock()
qdisc_graft()
// parent != NULL
->cops-graft()
notify_and_destroy()			qdisc_run()
					spin_lock(root_lock)
qdisc_destroy(old)			dequeue_skb()
					tbf_dequeue()
					qdisc_tree_decrease_qlen()	
					qdisc_lookup()
//deleting from qdisc_sleeping->list	//walking qdisc_sleeping->list
//under rtnl_lock() only		//under qdisc root_lock only
list_del(qdisc->list)			list_for_each_entry(txq_root)

Good catch.  Longer term we should fix it so that it doesn't do
the silly lookup at run-time.  In fact we'll be getting rid of
requeue so there will be no need to do this in TBF at all.

However, for now please create a patch to put a pair of root locks
around the list_del in qdisc_destroy.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt