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

[Andrey Borzenkov <arvidjaar@mail.ru>]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

On 8 января 2009 21:24:16 Dave wrote:
> 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.
>

What exactly prevents crypto library being called from interrupt (aka 
can it sleep)? Because now it runs under spinlock with interrupts 
disabled; how exactly does it differ from being called from interrupt 
context?

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

For all I can tell, code now is functionally identical to code before 
moving rx processing to tasklet. Meaning, if it was wrong before it 
became just as wrong now.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]