Re: [ 67/75] ipw2200: Fix race condition in the command completionacknowledge

[Ben Hutchings]

On Mon, 2012-05-07 at 16:45 -0700, Stanislav Yakovlev wrote:
> On 7 May 2012 07:37, Ben Hutchings <ben@xxxxxxxxxxxxxxx> wrote:
> > On Fri, 2012-05-04 at 13:43 -0700, Greg KH wrote:
> >> 3.3-stable review patch.  If anyone has any objections, please let me know.
> >>
> >> ------------------
> >>
> >> From: Stanislav Yakovlev <stas.yakovlev@xxxxxxxxx>
> >>
> >> commit dd447319895d0c0af423e483d9b63f84f3f8869a upstream.
> >>
> >> Driver incorrectly validates command completion: instead of waiting
> >> for a command to be acknowledged it continues execution.  Most of the
> >> time driver gets acknowledge of the command completion in a tasklet
> >> before it executes the next one. But sometimes it sends the next
> >> command before it gets acknowledge for the previous one. In such a
> >> case one of the following error messages appear in the log:
> > [...]
> >> +     now = jiffies;
> >> +     end = now + HOST_COMPLETE_TIMEOUT;
> >> +again:
> >>       rc = wait_event_interruptible_timeout(priv->wait_command_queue,
> >>                                             !(priv->
> >>                                               status & STATUS_HCMD_ACTIVE),
> >> -                                           HOST_COMPLETE_TIMEOUT);
> >> +                                           end - now);
> >> +     if (rc < 0) {
> >> +             now = jiffies;
> >> +             if (time_before(now, end))
> >> +                     goto again;
> >> +             rc = 0;
> >> +     }
> > [...]
> >
> > If you don't want the wait to be interrupted, use wait_event_timeout()
> > instead of this ridiculous loop!
> 
> Usually "modprobe ipw2200" takes less than a second, and after
> switching to wait_event_timeout it will take more than 10 seconds. We
> can not decrease the waiting timeout because we want to keep it big
> enough for the worst case(when the firmware did not reply).
> 
> There is a relevant discussion on lklm:
> https://lkml.org/lkml/2008/9/25/2
> https://lkml.org/lkml/2008/9/26/194

That's irrelevant: you aren't letting userland handle the signal here,
so you should use wait_event_timeout() and no loop.

Ben.

-- 
Ben Hutchings
Life is what happens to you while you're busy making other plans.
                                                               - John Lennon

Attachment: signature.asc
Description: This is a digitally signed message part