Thread (87 messages) 87 messages, 5 authors, 2012-05-09

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

From: Ben Hutchings <hidden>
Date: 2012-05-08 00:14:26
Also in: lkml

On Mon, 2012-05-07 at 16:45 -0700, Stanislav Yakovlev wrote:
On 7 May 2012 07:37, Ben Hutchings [off-list ref] wrote:
quoted
On Fri, 2012-05-04 at 13:43 -0700, Greg KH wrote:
quoted
3.3-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Stanislav Yakovlev <stas.yakovlev@gmail.com>

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:
[...]
quoted
+     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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help