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,
Thanks,
Javier
--
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
