On Sun, Aug 15, 2004 at 07:58:20PM -0700, David S. Miller wrote:

On Sat, 14 Aug 2004 10:34:28 +1000
Herbert Xu [off-list ref] wrote:

quoted

BTW, it looks like you can remove inetdev_lock altogether.  All its
users already assume that they don't race against each other by
taking the rtnl lock.

I think it's needed at least for the:

	dev->ip_ptr = idev;
	in_dev_get(idev);

You mean in_dev_hold?

thing, isn't it?

I don't think so.  But there is a real bug here :)

The lock doesn't really do much since everyone who's holding
it (they're all in devinet.c) already hold the RTNL lock
(the rotten old lock perhaps :)

But the RCU change has created a real bug here.  Imagine this:

CPU0					CPU1
inetdev_init
	dev->ip_ptr = idev
					in_dev_get
						atomic_inc on refcnt

					in_dev_put => frees idev
	in_dev_hold(idev) => BUG

So we should reverse the two statements and add an smp_wmb().

Unfortunately the lock doesn't really help you at all.

Cheers,
-- 
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