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:8.518 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site