Re: [PATCH 2/2] mac80211: Fixing Races for skipping tailroom reservation

[yogeshp <yogeshp@marvell.com>]

On Wednesday 22 June 2011 06:42 PM, Johannes Berg wrote:
> On Wed, 2011-06-22 at 18:28 +0530, Yogesh Ashok Powar wrote:
>> On Wed, Jun 22, 2011 at 05:49:25AM -0700, Johannes Berg wrote:
>>> On Wed, 2011-06-22 at 18:01 +0530, Yogesh Ashok Powar wrote:
>>>>> Will work on some other logic.
>>>>
>>>> Following is the complete V2-patch
>>>>
>>>> v2 changes: a) Moved counter++ before __ieee80211_key_replace in
>>>> 		key_link()
>>>> 	    b) Moved crypto_tx_tailroom_needed_cnt to sdata resolve
>>>> 	        issue with multiple sdata instances in hw reset.
>>>
>>> Looks good. Now I'm just worried about memory and compiler barriers that
>>> may be needed so the counter update doesn't move after anything else...
>>> Hmm.
>>
>> I some how feel that synchronize_net may be replaced with
>> synchronize_rcu and still the race wont happen.
>>
>> So the new logic would be 
>> - counter++		<-- Here keys are not added or deleted
>> 			    so rcu readers wont have problem
>> 			    with extra space allocated.
>>
>> - synchronize_rcu	<-- This will flush existing readers. Again
>> 			    new readers will have no problem with extra
>> 			    space allocated.
>>
>> Let me know your opinion on this.
> 
> It doesn't really matter -- synchronize_net() is exactly the same as
> synchronize_rcu().
> 
> What I'm worried about is that there's no memory barrier after the
> counter update, so how do we know it cannot happen after
> synchronize_rcu()? I think we need something like rcu_assign_pointer()
> but I don't see rcu_assign_index() (any more?)

Hi Johannes,
 Andreas has tested the current implementation on his setup and haven't
seen any WARN_ONs being hit. Should I send the final patch? 

Thanks
Yogesh