Re: [RFC][PATCH] rcu in drivers/net/hamradio

[Herbert Xu <herbert@gondor.apana.org.au>]

On Fri, Oct 14, 2005 at 09:38:54AM -0700, Suzanne Wood wrote:
> 
> ChangeLog:
> Because bpq_new_device() calls list_add_rcu()
> and bpq_free_device() calls list_del_rcu(),
> substitute list_for_each_entry_rcu() for 
> list_for_each_entry() in bpq_get_ax25_dev().
> This requires the insertion of rcu_read_lock()/unlock().

The rcu_read_lock/unlock is unnecessary because the only caller that
doesn't hold RTNL (bpq_rcv) already takes that lock.  In fact it's
better to take it there since you need to hold the lock for the duration
of the use of the device.

> A consequence of list_for_each_entry_rcu(bpq, &bpq_devices, bpq_list)
> is that the future dereference of the pointer to the bpqdev 
> struct bpq is rcu-protected.  But bpq_get_ax25_dev() 
> returns bpq->axdev, a pointer to a net_device struct.  The 
> rcu_read_lock() in bpq_rcv() likely implies that is another 
> pointer to receive rcu-protected dereference.

The rcu_dereference should be provided by list_for_each_entry_rcu.
In fact there is a bug in the list_*_rcu macros where the first
element is not put through rcu_dereference.  I'll fix that up.

> The rcu_read_lock()/unlock() in bpq_device_event() 
> are removed due to the following found by considering 
> the cases of the switch statement: 

Agreed.

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