[PATCH] Fix drain_console unregistration

[Peter Hutterer]

sigh. I had a fix for this back in August but it never made it into master
http://lists.freedesktop.org/archives/xorg-devel/2011-August/024354.html

On Tue, Oct 11, 2011 at 09:11:18AM +0200, Tomáš Trnka wrote:
> Bug introduced by 9dca441670d261a9a9fb6108960ed48f3d58fb7f
> xfree86: add a hook to replace the new console handler.
> 
> console_handler was not being set, making the server eat up CPU spinning
> in WaitForSomething selecting consoleFd over and over again, every time
> trying to unregister drain_console without success due to
> console_handler being NULL.
> 
> Let's just fix the unregistration in xf86SetConsoleHandler() and use that.
> 
> But wait, there could be a catch: If some driver replaced the handler using
> xf86SetConsoleHandler(), the unregistration in xf86CloseConsole will unregister
> that one. I don't understand Xorg well enough to know whether this poses a
> problem (could mess up driver deinit somehow or something like that). As it is,
> xf86SetConsoleHandler() doesn't offer any way to prevent this (i.e. check which
> handler is currently registered).

I don't think we really handle multiple console handlers so we probably
don't need to worry about this.

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>, I've merged this
into my tree. thanks.

Cheers,
  Peter

> I had been using it for two days on my machine that previously hit 100% CPU
> several times a day. That has now gone away without any new problems appearing.
> 
> Signed-off-by: Tomas Trnka <tomastrnka at gmx.com>
> ---
>  hw/xfree86/common/xf86Events.c         |    9 ++++-----
>  hw/xfree86/os-support/linux/lnx_init.c |   14 ++++++--------
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/xfree86/common/xf86Events.c b/hw/xfree86/common/xf86Events.c
> index 84c0d18..8511ea1 100644
> --- a/hw/xfree86/common/xf86Events.c
> +++ b/hw/xfree86/common/xf86Events.c
> @@ -603,16 +603,15 @@ xf86AddGeneralHandler(int fd, InputHandlerProc proc, pointer data)
>  InputHandlerProc
>  xf86SetConsoleHandler(InputHandlerProc proc, pointer data)
>  {
> -    static InputHandlerProc handler = NULL;
> -    InputHandlerProc old_handler = handler;
> +    static IHPtr handler = NULL;
> +    IHPtr old_handler = handler;
>  
>      if (old_handler)
>          xf86RemoveGeneralHandler(old_handler);
>  
> -    xf86AddGeneralHandler(xf86Info.consoleFd, proc, data);
> -    handler = proc;
> +    handler = xf86AddGeneralHandler(xf86Info.consoleFd, proc, data);
>  
> -    return old_handler;
> +    return (old_handler) ? old_handler->ihproc : NULL;
>  }
>  
>  static void
> diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c
> index 77dfb2f..adcc467 100644
> --- a/hw/xfree86/os-support/linux/lnx_init.c
> +++ b/hw/xfree86/os-support/linux/lnx_init.c
> @@ -47,15 +47,12 @@ static char vtname[11];
>  static struct termios tty_attr; /* tty state to restore */
>  static int tty_mode; /* kbd mode to restore */
>  
> -static void *console_handler;
> -
>  static void
>  drain_console(int fd, void *closure)
>  {
>      errno = 0;
>      if (tcflush(fd, TCIOFLUSH) == -1 && errno == EIO) {
> -	xf86RemoveGeneralHandler(console_handler);
> -	console_handler = NULL;
> +        xf86SetConsoleHandler(NULL, NULL);
>      }
>  }
>  
> @@ -259,10 +256,11 @@ xf86CloseConsole(void)
>          return;
>      }
>  
> -    if (console_handler) {
> -	xf86RemoveGeneralHandler(console_handler);
> -	console_handler = NULL;
> -    };
> +    /*
> +     * unregister the drain_console handler
> +     * - what to do if someone else changed it in the meantime?
> +     */
> +    xf86SetConsoleHandler(NULL, NULL);
>  
>      /* Back to text mode ... */
>      SYSCALL(ret = ioctl(xf86Info.consoleFd, KDSETMODE, KD_TEXT));
> -- 
> 1.7.6.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>