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    [from the cache]
©2003-2011 Jasper Spaans