This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [RFC PATCH 27/33] Add the Xen virtual console driver.

On 18 Jul 2006, at 11:24, Arjan van de Ven wrote:

hmm somehow I find this code scary; we had similar code recently
elsewhere where this turned out to be a real issue; you now sleep for
"1" time, so you sleep for a fixed time if you aren't getting wakeups,
but if you are getting wakeups your code is upside down, I would expect
it to look like

+       set_current_state(TASK_INTERRUPTIBLE);
+       while (DRV(tty->driver)->chars_in_buffer(tty))
+               schedule_timeout(1);
+               if (signal_pending(current))
+                       break;
+               if (timeout && time_after(jiffies, orig_jiffies + timeout))
+                       break;
+               set_current_state(TASK_INTERRUPTIBLE);
+       }

instead, so that you don't have the wakeup race..

There's no wakeup signal, so no possibility of a wakeup race. That's why we schedule_timeout() instead of wait_event() or similar. This code is only used to flush the console when the kernel crashes, so we can get the full oops, so waiting a little bit too long is acceptable.

Your suggested change is perhaps more idiomatic though, and less jarring for reviewers. :-)

Thanks for your comments by the way. Reviewing lots of patches isn't much fun.

 -- Keir

Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>