lkml.org 
[lkml]   [2007]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 8/8] Char: vt_ioctl, use wait_event_interruptible
    On Mon, 28 May 2007 15:31:12 +0200 (CEST)
    Jiri Slaby <jirislaby@gmail.com> wrote:

    > vt_ioctl, use wait_event_interruptible
    >
    > Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
    >
    > ---
    > commit fbe1931e02f11b2fef771ff1698f1598b3567520
    > tree fcfcd72a5619f6e26598ac2ee3132aed4b070987
    > parent c025c4b3eca99f50b05bc24c445b861e91226539
    > author Jiri Slaby <jirislaby@gmail.com> Sat, 26 May 2007 22:58:17 +0200
    > committer Jiri Slaby <jirislaby@gmail.com> Sat, 26 May 2007 22:58:17 +0200
    >
    > drivers/char/vt_ioctl.c | 22 ++++------------------
    > 1 files changed, 4 insertions(+), 18 deletions(-)
    >
    > diff --git a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
    > index c6f6f42..2056367 100644
    > --- a/drivers/char/vt_ioctl.c
    > +++ b/drivers/char/vt_ioctl.c
    > @@ -1035,12 +1035,8 @@ static DECLARE_WAIT_QUEUE_HEAD(vt_activate_queue);
    > int vt_waitactive(int vt)
    > {
    > int retval;
    > - DECLARE_WAITQUEUE(wait, current);
    > -
    > - add_wait_queue(&vt_activate_queue, &wait);
    > - for (;;) {
    > - retval = 0;
    >
    > + return wait_event_interruptible(vt_activate_queue, ({
    > /*
    > * Synchronize with redraw_screen(). By acquiring the console
    > * semaphore we make sure that the console switch is completed
    > @@ -1049,20 +1045,10 @@ int vt_waitactive(int vt)
    > * updated, but the console switch hasn't been completed.
    > */
    > acquire_console_sem();
    > - set_current_state(TASK_INTERRUPTIBLE);
    > - if (vt == fg_console) {
    > - release_console_sem();
    > - break;
    > - }
    > + retval = vt == fg_console;
    > release_console_sem();
    > - retval = -EINTR;
    > - if (signal_pending(current))
    > - break;
    > - schedule();
    > - }
    > - remove_wait_queue(&vt_activate_queue, &wait);
    > - __set_current_state(TASK_RUNNING);
    > - return retval;
    > + retval;
    > + }));
    > }
    >

    So we end up with

    int vt_waitactive(int vt)
    {
    int retval;

    return wait_event_interruptible(vt_activate_queue, ({
    /*
    * Synchronize with redraw_screen(). By acquiring the console
    * semaphore we make sure that the console switch is completed
    * before we return. If we didn't wait for the semaphore, we
    * could return at a point where fg_console has already been
    * updated, but the console switch hasn't been completed.
    */
    acquire_console_sem();
    retval = vt == fg_console;
    release_console_sem();
    retval;
    }));
    }

    Again, I do think this needs a helper function. Or something. The handling
    of `retval' in there is pretty perverse. We're modifying a local variable
    within the macro and then returning it by value? Perhaps a bit cleaner
    would be to move `retval' inside the macro body.

    But a helper function would be better. Again, remember that the macro
    evaluates that expression twice - you may find that the helper function
    even generates less code.

    -
    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: 2007-05-30 02:11    [W:0.062 / U:93.860 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site