lkml.org 
[lkml]   [2006]   [Jul]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH 27/33] Add the Xen virtual console driver.
Date

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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-07-18 12:35    [W:1.253 / U:0.620 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site