On Thu, 25 Aug 2011 11:45:11 -0700, Javier Cardona wrote:
On Thu, Aug 25, 2011 at 11:21 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
On Thu, 25 Aug 2011 11:16:50 -0700, Javier Cardona wrote:
On Wed, Aug 24, 2011 at 7:08 PM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
On Wed, 24 Aug 2011 18:40:44 -0700, Thomas Pedersen wrote:
da = hdr->addr3;
ra = hdr->addr1;
+ rcu_read_lock();
mpath = mesh_path_lookup(da, sdata);
+ rcu_read_unlock();
if (mpath)
sn = ++mpath->sn;
mesh_path_error_tx(sdata->u.mesh.mshcfg.element_ttl,
skb->data,
You've got to be kidding. Didn't I just explain RCU :)
The patch was prepared before your RCU session :(
Just to confirm I got it right before we resubmit: given that not
only
the path table accessed inside mesh_path_lookup() but also the
mpaths
themselves are RCU protected, the right fix should have been
da = hdr->addr3;
ra = hdr->addr1;
+ rcu_read_lock();
mpath = mesh_path_lookup(da, sdata);
if (mpath)
sn = ++mpath->sn;
+ rcu_read_unlock();
mesh_path_error_tx(sdata->u.mesh.mshcfg.element_ttl,
skb->data,
Correct?
Frankly, I'm not sure, since you modify the mpath->sn you probably
need to
hold a real lock, otherwise ++mpath->sn can race against itself in
this very
function.
Oh, I see. That's a different issue from what I was originally
trying
to fix (unprotected access to the path table inside the lookup
function). But you are completely right. Changing the math sequence
number requires taking the mpath state lock:
da = hdr->addr3;
ra = hdr->addr1;
+ rcu_read_lock();
mpath = mesh_path_lookup(da, sdata);
- if (mpath)
+ if (mpath) {
+ spin_lock_bh(&mpath->state_lock);
sn = ++mpath->sn;
+ spin_unlock_bh(&mpath->state_lock);
+ }
+ rcu_read_unlock();
mesh_path_error_tx(sdata->u.mesh.mshcfg.element_ttl,
skb->data,
Seems about right to me, but I don't know about all the locking :)
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
