Thank you and sorry if I'm belaboring this.
> From [email protected] Fri Oct 14 15:36:36 2005
> 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.
Thought this was an example of protecting a pointer dereference of an object
and then wanting deferred destruction to extend to a field of that object
which is pointer to a different object, similar to traversing a list.
So I guess the dereference being protected is bpq->axdev and not a deref
of that pointer to the net_device struct, since we see the fields of dev
being assigned within the rcu read-side critical section. Thank you for
correcting me on this.
> 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.
This is very significant.
> > 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.
Another list_for_each_entry() in bpq_seq_start() in a marked rcu
read-side critical section becomes the rcu version.
Please consider a corrected patch attached.
Thank you.
-------
bpqether.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
-------
--- src/linux-2.6.14-rc4/drivers/net/hamradio/bpqether.c 2005-10-10 18:19:19.000000000 -0700
+++ patch/linux-2.6.14-rc4/drivers/net/hamradio/bpqether.c 2005-10-15 00:42:14.000000000 -0700
@@ -144,7 +144,7 @@ static inline struct net_device *bpq_get
{
struct bpqdev *bpq;
- list_for_each_entry(bpq, &bpq_devices, bpq_list) {
+ list_for_each_entry_rcu(bpq, &bpq_devices, bpq_list) {
if (bpq->ethdev == dev)
return bpq->axdev;
}
@@ -399,7 +399,7 @@ static void *bpq_seq_start(struct seq_fi
if (*pos == 0)
return SEQ_START_TOKEN;
- list_for_each_entry(bpqdev, &bpq_devices, bpq_list) {
+ list_for_each_entry_rcu(bpqdev, &bpq_devices, bpq_list) {
if (i == *pos)
return bpqdev;
}
@@ -561,8 +561,6 @@ static int bpq_device_event(struct notif
if (!dev_is_ethdev(dev))
return NOTIFY_DONE;
- rcu_read_lock();
-
switch (event) {
case NETDEV_UP: /* new ethernet device -> new BPQ interface */
if (bpq_get_ax25_dev(dev) == NULL)
@@ -581,7 +579,6 @@ static int bpq_device_event(struct notif
default:
break;
}
- rcu_read_unlock();
return NOTIFY_DONE;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
