Re: [PATCH] orinoco: take the driver lock in the rx tasklet

[Dave <kilroyd@googlemail.com>]

Andrey Borzenkov wrote:
> On 07 January 2009 03:23:55 David Kilroy wrote:
>> Fix the warning reproduced below.
>>
>> We add to rx_list in interrupt context and remove elements in tasklet
>> context. While removing elements we need to prevent the interrupt
>> modifying the list.
>>
>> Note that commit 31afcef385bb8bf528c6fbe05b359af9f456f02a did not
>> preserve locking semantics on what is now orinoco_rx.
>>
>> This patch reinstates the locking semantics and ensures it covers
>> rx_list as well. 
> [...]
>> --- a/drivers/net/wireless/orinoco/orinoco.c
>> +++ b/drivers/net/wireless/orinoco/orinoco.c
>> @@ -1613,6 +1613,16 @@ static void orinoco_rx_isr_tasklet(unsigned
> [...]
>> +	if (orinoco_lock(priv, &flags) != 0)
>> +		return;
>>
> 
> this effectively serializes rx list processing with the rest of driver 
> (specifically orinoco_interrupt) eliminating any benefit from doing it 
> in separate tasklet. It is more simple and less confusing to just revert 
> commit 31afcef385bb8bf528c6fbe05b359af9f456f02a then.

[adding orinoco-devel, who I missed of the original patch submission]

The move to the RX tasklet was not motivated by improved performance,
but the requirements of the crypto library (can't call from interrupt)
and the need to do MIC calculations on RX.

Unless the crypto routines have changed, reverting that commit will
break WPA support.



Regards,

Dave.