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

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

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
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel