Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Ingo Molnar

* Jan Luebbe <[EMAIL PROTECTED]> wrote:

> I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using 
> psplash and kdrive. When booting, I got an error from psplash about 
> being unable to switch consoles. Strangely, it was possible to start 
> kdrive manually. So i set up qemu for kernel debugging, set some 
> interesting breakpoints and then saw that it was scheduling away 
> directly after the kill_pid. Then it was quite obvious.
> 
> It was triggering every time, on the device and in qemu identically. 
> Most people testing the RCs probably don't run psplash (or any other 
> program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed 
> earlier.

thx for the description. I did some searching on google.com/codesearch, 
and here are some other console apps that make active use of VT_PROCESS:

   1diskxwin-2.0.0/X11/programs/Xserver/hw/kdrive/linux/linux.c
   usr/include/dev/wscons/wsdisplay_compat_usl.c
   busybox-1.01/loginutils/vlock.c
   allegro-4.2.2/src/linux/vtswitch.c
   tvision/classes/linux/linuxkey.cc
   xc/programs/Xserver/hw/xfree86/os-support/pmax/pmax_init.c
   aalib-1.4.0/src/aalinuxkbd.c
   qt-embedded-opensource-4.0.0-b2/src/gui/embedded/qkbdtty_qws.cpp
   mc-4.5.55/src/cons.handler.c
   psplash-0.1/psplash-console.c
   quakeforge-0.5.5/libs/video/targets/vid_fbdev.c

just in case someone has a regression with any of them, on an older 
kernel. To me it appears X should have triggered this too. Perhaps 
people dont switch between VGA and X all that frequently anymore ...

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


Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Jan Luebbe
On Sun, 2007-09-30 at 09:20 +0200, Ingo Molnar wrote:
> * Jan Luebbe <[EMAIL PROTECTED]> wrote:
> > From: Jan Lübbe <[EMAIL PROTECTED]>
> > 
> > The new behaviour of CFS exposes a race which occurs if a switch is
> > requested when vt_mode.mode is VT_PROCESS.
> > 
> > The process with vc->vt_pid is signaled before vc->vt_newvt is set. This
> > causes the switch to fail when triggered by the monitoing process
> > because the target is still -1.
> > 
> > Signed-off-by: Jan Lübbe <[EMAIL PROTECTED]>
> 
> nifty! I'm wondering what the symptoms of this bug were - how did you 
> notice it? Also, i'm wondering how you managed to debug this - looks 
> quite hard to trigger this race, right?
> 
> Acked-by: Ingo Molnar <[EMAIL PROTECTED]>
> 
>   Ingo

I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using
psplash and kdrive. When booting, I got an error from psplash about
being unable to switch consoles. Strangely, it was possible to start
kdrive manually. So i set up qemu for kernel debugging, set some
interesting breakpoints and then saw that it was scheduling away
directly after the kill_pid. Then it was quite obvious.

It was triggering every time, on the device and in qemu identically.
Most people testing the RCs probably don't run psplash (or any other
program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed
earlier.

Jan

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


Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Ingo Molnar

* Jan Luebbe <[EMAIL PROTECTED]> wrote:

> From: Jan Lübbe <[EMAIL PROTECTED]>
> 
> The new behaviour of CFS exposes a race which occurs if a switch is
> requested when vt_mode.mode is VT_PROCESS.
> 
> The process with vc->vt_pid is signaled before vc->vt_newvt is set. This
> causes the switch to fail when triggered by the monitoing process
> because the target is still -1.
> 
> Signed-off-by: Jan Lübbe <[EMAIL PROTECTED]>

nifty! I'm wondering what the symptoms of this bug were - how did you 
notice it? Also, i'm wondering how you managed to debug this - looks 
quite hard to trigger this race, right?

Acked-by: Ingo Molnar <[EMAIL PROTECTED]>

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


Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Ingo Molnar

* Jan Luebbe [EMAIL PROTECTED] wrote:

 From: Jan Lübbe [EMAIL PROTECTED]
 
 The new behaviour of CFS exposes a race which occurs if a switch is
 requested when vt_mode.mode is VT_PROCESS.
 
 The process with vc-vt_pid is signaled before vc-vt_newvt is set. This
 causes the switch to fail when triggered by the monitoing process
 because the target is still -1.
 
 Signed-off-by: Jan Lübbe [EMAIL PROTECTED]

nifty! I'm wondering what the symptoms of this bug were - how did you 
notice it? Also, i'm wondering how you managed to debug this - looks 
quite hard to trigger this race, right?

Acked-by: Ingo Molnar [EMAIL PROTECTED]

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


Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Jan Luebbe
On Sun, 2007-09-30 at 09:20 +0200, Ingo Molnar wrote:
 * Jan Luebbe [EMAIL PROTECTED] wrote:
  From: Jan Lübbe [EMAIL PROTECTED]
  
  The new behaviour of CFS exposes a race which occurs if a switch is
  requested when vt_mode.mode is VT_PROCESS.
  
  The process with vc-vt_pid is signaled before vc-vt_newvt is set. This
  causes the switch to fail when triggered by the monitoing process
  because the target is still -1.
  
  Signed-off-by: Jan Lübbe [EMAIL PROTECTED]
 
 nifty! I'm wondering what the symptoms of this bug were - how did you 
 notice it? Also, i'm wondering how you managed to debug this - looks 
 quite hard to trigger this race, right?
 
 Acked-by: Ingo Molnar [EMAIL PROTECTED]
 
   Ingo

I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using
psplash and kdrive. When booting, I got an error from psplash about
being unable to switch consoles. Strangely, it was possible to start
kdrive manually. So i set up qemu for kernel debugging, set some
interesting breakpoints and then saw that it was scheduling away
directly after the kill_pid. Then it was quite obvious.

It was triggering every time, on the device and in qemu identically.
Most people testing the RCs probably don't run psplash (or any other
program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed
earlier.

Jan

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


Re: [PATCH] fix console change race exposed by CFS

2007-09-30 Thread Ingo Molnar

* Jan Luebbe [EMAIL PROTECTED] wrote:

 I'm currenly porting the OpenMoko patchset to 2.6.23-rc8. We are using 
 psplash and kdrive. When booting, I got an error from psplash about 
 being unable to switch consoles. Strangely, it was possible to start 
 kdrive manually. So i set up qemu for kernel debugging, set some 
 interesting breakpoints and then saw that it was scheduling away 
 directly after the kill_pid. Then it was quite obvious.
 
 It was triggering every time, on the device and in qemu identically. 
 Most people testing the RCs probably don't run psplash (or any other 
 program which sets vt_mode.mode to VT_PROCESS), so it wasn't noticed 
 earlier.

thx for the description. I did some searching on google.com/codesearch, 
and here are some other console apps that make active use of VT_PROCESS:

   1diskxwin-2.0.0/X11/programs/Xserver/hw/kdrive/linux/linux.c
   usr/include/dev/wscons/wsdisplay_compat_usl.c
   busybox-1.01/loginutils/vlock.c
   allegro-4.2.2/src/linux/vtswitch.c
   tvision/classes/linux/linuxkey.cc
   xc/programs/Xserver/hw/xfree86/os-support/pmax/pmax_init.c
   aalib-1.4.0/src/aalinuxkbd.c
   qt-embedded-opensource-4.0.0-b2/src/gui/embedded/qkbdtty_qws.cpp
   mc-4.5.55/src/cons.handler.c
   psplash-0.1/psplash-console.c
   quakeforge-0.5.5/libs/video/targets/vid_fbdev.c

just in case someone has a regression with any of them, on an older 
kernel. To me it appears X should have triggered this too. Perhaps 
people dont switch between VGA and X all that frequently anymore ...

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


[PATCH] fix console change race exposed by CFS

2007-09-29 Thread Jan Luebbe
From: Jan Lübbe <[EMAIL PROTECTED]>

The new behaviour of CFS exposes a race which occurs if a switch is
requested when vt_mode.mode is VT_PROCESS.

The process with vc->vt_pid is signaled before vc->vt_newvt is set. This
causes the switch to fail when triggered by the monitoing process
because the target is still -1.

Signed-off-by: Jan Lübbe <[EMAIL PROTECTED]>
---
Index: linux-2.6.22/drivers/char/vt_ioctl.c
===
--- linux-2.6.22.orig/drivers/char/vt_ioctl.c
+++ linux-2.6.22/drivers/char/vt_ioctl.c
@@ -1208,15 +1208,18 @@
/*
 * Send the signal as privileged - kill_pid() will
 * tell us if the process has gone or something else
-* is awry
+* is awry.
+*
+* We need to set vt_newvt *before* sending the signal or we
+* have a race.
 */
+   vc->vt_newvt = new_vc->vc_num;
if (kill_pid(vc->vt_pid, vc->vt_mode.relsig, 1) == 0) {
/*
 * It worked. Mark the vt to switch to and
 * return. The process needs to send us a
 * VT_RELDISP ioctl to complete the switch.
 */
-   vc->vt_newvt = new_vc->vc_num;
return;
}


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


[PATCH] fix console change race exposed by CFS

2007-09-29 Thread Jan Luebbe
From: Jan Lübbe [EMAIL PROTECTED]

The new behaviour of CFS exposes a race which occurs if a switch is
requested when vt_mode.mode is VT_PROCESS.

The process with vc-vt_pid is signaled before vc-vt_newvt is set. This
causes the switch to fail when triggered by the monitoing process
because the target is still -1.

Signed-off-by: Jan Lübbe [EMAIL PROTECTED]
---
Index: linux-2.6.22/drivers/char/vt_ioctl.c
===
--- linux-2.6.22.orig/drivers/char/vt_ioctl.c
+++ linux-2.6.22/drivers/char/vt_ioctl.c
@@ -1208,15 +1208,18 @@
/*
 * Send the signal as privileged - kill_pid() will
 * tell us if the process has gone or something else
-* is awry
+* is awry.
+*
+* We need to set vt_newvt *before* sending the signal or we
+* have a race.
 */
+   vc-vt_newvt = new_vc-vc_num;
if (kill_pid(vc-vt_pid, vc-vt_mode.relsig, 1) == 0) {
/*
 * It worked. Mark the vt to switch to and
 * return. The process needs to send us a
 * VT_RELDISP ioctl to complete the switch.
 */
-   vc-vt_newvt = new_vc-vc_num;
return;
}


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