* [PATCH] atkbd: cleanup only once
@ 2007-06-27 10:59 Dave Young
2007-06-27 3:01 ` dave young
2007-06-27 4:34 ` Dmitry Torokhov
0 siblings, 2 replies; 10+ messages in thread
From: Dave Young @ 2007-06-27 10:59 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-kernel
Hi,
If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer.
2.6.22-rc5 and 2.6.22-rc6 have same result.
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
---
diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c
--- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000
+++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000
@@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd *
static void atkbd_cleanup(struct serio *serio)
{
+ static int flag;
+
+ if(flag)
+ return;
+ flag = 1;
struct atkbd *atkbd = serio_get_drvdata(serio);
ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_BAT);
}
Regards
dave
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] atkbd: cleanup only once 2007-06-27 10:59 [PATCH] atkbd: cleanup only once Dave Young @ 2007-06-27 3:01 ` dave young 2007-06-27 4:34 ` Dmitry Torokhov 1 sibling, 0 replies; 10+ messages in thread From: dave young @ 2007-06-27 3:01 UTC (permalink / raw) To: dmitry.torokhov; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 1047 bytes --] 2007/6/27, Dave Young <hidave.darkstar@gmail.com>: > Hi, > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > --- > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > static void atkbd_cleanup(struct serio *serio) > { > + static int flag; > + > + if(flag) > + return; > + flag = 1; > struct atkbd *atkbd = serio_get_drvdata(serio); > ps2_command(&atkbd->ps2dev, NULL, ATKBD_CMD_RESET_BAT); > } > > Regards > dave > attached please find the oops screen image. [-- Attachment #2: screen.jpg --] [-- Type: image/jpeg, Size: 117541 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 10:59 [PATCH] atkbd: cleanup only once Dave Young 2007-06-27 3:01 ` dave young @ 2007-06-27 4:34 ` Dmitry Torokhov 2007-06-27 4:28 ` Greg KH 1 sibling, 1 reply; 10+ messages in thread From: Dmitry Torokhov @ 2007-06-27 4:34 UTC (permalink / raw) To: Dave Young, Greg KH; +Cc: linux-kernel Hi Dave, On Wednesday 27 June 2007 06:59, Dave Young wrote: > Hi, > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > --- > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > static void atkbd_cleanup(struct serio *serio) > { > + static int flag; > + > + if(flag) > + return; > + flag = 1; Unfortunately this will prevent atkbd from resetting keyboard on 2nd suspend attempt. It will also not work if you have an active MUX and have a couple of keyboards connected. Greg, now that you removed rwsem from subsystem (and subsystem itself for that matter) there is nothing as far as I can see that stops several threads from running device_shutdown() simultaneously. I also do not see what would isolate device probing and shutting them down at the same time. Am I missing something? -- Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 4:34 ` Dmitry Torokhov @ 2007-06-27 4:28 ` Greg KH 2007-06-27 4:59 ` Dmitry Torokhov 0 siblings, 1 reply; 10+ messages in thread From: Greg KH @ 2007-06-27 4:28 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Dave Young, linux-kernel On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > Hi Dave, > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > Hi, > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > --- > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > static void atkbd_cleanup(struct serio *serio) > > { > > + static int flag; > > + > > + if(flag) > > + return; > > + flag = 1; > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > suspend attempt. It will also not work if you have an active MUX and > have a couple of keyboards connected. > > Greg, now that you removed rwsem from subsystem (and subsystem itself > for that matter) there is nothing as far as I can see that stops > several threads from running device_shutdown() simultaneously. I also > do not see what would isolate device probing and shutting them down > at the same time. Am I missing something? There was never anything stopping that from happening before. No driver core code was using that rwsem, so it wasn't protecting anything, despite people trying to use it as if it was :) That's why I removed it. So, if you need to have a lock for your subsystem to serialize this, please do so, I have no objection to it. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 4:28 ` Greg KH @ 2007-06-27 4:59 ` Dmitry Torokhov 2007-06-27 5:02 ` dave young 2007-06-27 5:27 ` Greg KH 0 siblings, 2 replies; 10+ messages in thread From: Dmitry Torokhov @ 2007-06-27 4:59 UTC (permalink / raw) To: Greg KH; +Cc: Dave Young, linux-kernel On Wednesday 27 June 2007 00:28, Greg KH wrote: > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > Hi Dave, > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > Hi, > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > --- > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > static void atkbd_cleanup(struct serio *serio) > > > { > > > + static int flag; > > > + > > > + if(flag) > > > + return; > > > + flag = 1; > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > suspend attempt. It will also not work if you have an active MUX and > > have a couple of keyboards connected. > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > for that matter) there is nothing as far as I can see that stops > > several threads from running device_shutdown() simultaneously. I also > > do not see what would isolate device probing and shutting them down > > at the same time. Am I missing something? > > There was never anything stopping that from happening before. No driver > core code was using that rwsem, so it wasn't protecting anything, > despite people trying to use it as if it was :) > It did protect device_shutdown() from itself, didn't it? -- Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 4:59 ` Dmitry Torokhov @ 2007-06-27 5:02 ` dave young 2007-06-28 5:18 ` Dmitry Torokhov 2007-06-27 5:27 ` Greg KH 1 sibling, 1 reply; 10+ messages in thread From: dave young @ 2007-06-27 5:02 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Greg KH, linux-kernel 2007/6/27, Dmitry Torokhov <dtor@insightbb.com>: > On Wednesday 27 June 2007 00:28, Greg KH wrote: > > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > > Hi Dave, > > > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > > Hi, > > > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > > --- > > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > > > static void atkbd_cleanup(struct serio *serio) > > > > { > > > > + static int flag; > > > > + > > > > + if(flag) > > > > + return; > > > > + flag = 1; > > > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > > suspend attempt. It will also not work if you have an active MUX and > > > have a couple of keyboards connected. > > > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > > for that matter) there is nothing as far as I can see that stops > > > several threads from running device_shutdown() simultaneously. I also > > > do not see what would isolate device probing and shutting them down > > > at the same time. Am I missing something? > > > > There was never anything stopping that from happening before. No driver > > core code was using that rwsem, so it wasn't protecting anything, > > despite people trying to use it as if it was :) > > > > It did protect device_shutdown() from itself, didn't it? > > -- > Dmitry > how about check ps2dev->serio in ps2_command before use it? Regards dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 5:02 ` dave young @ 2007-06-28 5:18 ` Dmitry Torokhov 2007-06-28 6:12 ` dave young 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Torokhov @ 2007-06-28 5:18 UTC (permalink / raw) To: dave young; +Cc: Greg KH, linux-kernel On Wednesday 27 June 2007 01:02, dave young wrote: > 2007/6/27, Dmitry Torokhov <dtor@insightbb.com>: > > On Wednesday 27 June 2007 00:28, Greg KH wrote: > > > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > > > Hi Dave, > > > > > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > > > Hi, > > > > > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > > > --- > > > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > > > > > static void atkbd_cleanup(struct serio *serio) > > > > > { > > > > > + static int flag; > > > > > + > > > > > + if(flag) > > > > > + return; > > > > > + flag = 1; > > > > > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > > > suspend attempt. It will also not work if you have an active MUX and > > > > have a couple of keyboards connected. > > > > > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > > > for that matter) there is nothing as far as I can see that stops > > > > several threads from running device_shutdown() simultaneously. I also > > > > do not see what would isolate device probing and shutting them down > > > > at the same time. Am I missing something? > > > > > > There was never anything stopping that from happening before. No driver > > > core code was using that rwsem, so it wasn't protecting anything, > > > despite people trying to use it as if it was :) > > > > > > > It did protect device_shutdown() from itself, didn't it? > > > > -- > > Dmitry > > > how about check ps2dev->serio in ps2_command before use it? I don't think we ever set it to NULL. Does the patch below help any? -- Dmitry Signed-off-by: Dmitry Torokhov <dtor@mail.ru> --- drivers/input/serio/serio.c | 2 ++ 1 files changed, 2 insertions(+) Index: work/drivers/input/serio/serio.c =================================================================== --- work.orig/drivers/input/serio/serio.c +++ work/drivers/input/serio/serio.c @@ -769,8 +769,10 @@ static int serio_driver_remove(struct de static void serio_cleanup(struct serio *serio) { + mutex_lock(&serio->drv_mutex); if (serio->drv && serio->drv->cleanup) serio->drv->cleanup(serio); + mutex_unlock(&serio->drv_mutex); } static void serio_shutdown(struct device *dev) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-28 5:18 ` Dmitry Torokhov @ 2007-06-28 6:12 ` dave young 0 siblings, 0 replies; 10+ messages in thread From: dave young @ 2007-06-28 6:12 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Greg KH, linux-kernel >2007/6/28, Dmitry Torokhov <dtor@insightbb.com>: > On Wednesday 27 June 2007 01:02, dave young wrote: > > 2007/6/27, Dmitry Torokhov <dtor@insightbb.com>: > > > On Wednesday 27 June 2007 00:28, Greg KH wrote: > > > > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > > > > Hi Dave, > > > > > > > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > > > > Hi, > > > > > > > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > > > > --- > > > > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > > > > > > > static void atkbd_cleanup(struct serio *serio) > > > > > > { > > > > > > + static int flag; > > > > > > + > > > > > > + if(flag) > > > > > > + return; > > > > > > + flag = 1; > > > > > > > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > > > > suspend attempt. It will also not work if you have an active MUX and > > > > > have a couple of keyboards connected. > > > > > > > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > > > > for that matter) there is nothing as far as I can see that stops > > > > > several threads from running device_shutdown() simultaneously. I also > > > > > do not see what would isolate device probing and shutting them down > > > > > at the same time. Am I missing something? > > > > > > > > There was never anything stopping that from happening before. No driver > > > > core code was using that rwsem, so it wasn't protecting anything, > > > > despite people trying to use it as if it was :) > > > > > > > > > > It did protect device_shutdown() from itself, didn't it? > > > > > > -- > > > Dmitry > > > > > how about check ps2dev->serio in ps2_command before use it? > > I don't think we ever set it to NULL. Does the patch below help any? > > -- > Dmitry > > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru> > --- > > drivers/input/serio/serio.c | 2 ++ > 1 files changed, 2 insertions(+) > > Index: work/drivers/input/serio/serio.c > =================================================================== > --- work.orig/drivers/input/serio/serio.c > +++ work/drivers/input/serio/serio.c > @@ -769,8 +769,10 @@ static int serio_driver_remove(struct de > > static void serio_cleanup(struct serio *serio) > { > + mutex_lock(&serio->drv_mutex); > if (serio->drv && serio->drv->cleanup) > serio->drv->cleanup(serio); > + mutex_unlock(&serio->drv_mutex); > } > > static void serio_shutdown(struct device *dev) > Yes, l tested with this patch, looks good to me. Regards dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 4:59 ` Dmitry Torokhov 2007-06-27 5:02 ` dave young @ 2007-06-27 5:27 ` Greg KH 2007-06-27 13:37 ` Dmitry Torokhov 1 sibling, 1 reply; 10+ messages in thread From: Greg KH @ 2007-06-27 5:27 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: Dave Young, linux-kernel On Wed, Jun 27, 2007 at 12:59:32AM -0400, Dmitry Torokhov wrote: > On Wednesday 27 June 2007 00:28, Greg KH wrote: > > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > > Hi Dave, > > > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > > Hi, > > > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > > --- > > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > > > static void atkbd_cleanup(struct serio *serio) > > > > { > > > > + static int flag; > > > > + > > > > + if(flag) > > > > + return; > > > > + flag = 1; > > > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > > suspend attempt. It will also not work if you have an active MUX and > > > have a couple of keyboards connected. > > > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > > for that matter) there is nothing as far as I can see that stops > > > several threads from running device_shutdown() simultaneously. I also > > > do not see what would isolate device probing and shutting them down > > > at the same time. Am I missing something? > > > > There was never anything stopping that from happening before. No driver > > core code was using that rwsem, so it wasn't protecting anything, > > despite people trying to use it as if it was :) > > > > It did protect device_shutdown() from itself, didn't it? Hm, yeah, it did, but that was it. If that was its goal, it sure wasn't obvious at all. Do you think the driver core needs to serialize this? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] atkbd: cleanup only once 2007-06-27 5:27 ` Greg KH @ 2007-06-27 13:37 ` Dmitry Torokhov 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Torokhov @ 2007-06-27 13:37 UTC (permalink / raw) To: Greg KH; +Cc: Dave Young, linux-kernel On 6/27/07, Greg KH <gregkh@suse.de> wrote: > On Wed, Jun 27, 2007 at 12:59:32AM -0400, Dmitry Torokhov wrote: > > On Wednesday 27 June 2007 00:28, Greg KH wrote: > > > On Wed, Jun 27, 2007 at 12:34:09AM -0400, Dmitry Torokhov wrote: > > > > Hi Dave, > > > > > > > > On Wednesday 27 June 2007 06:59, Dave Young wrote: > > > > > Hi, > > > > > > > > > > If you press ctrl+alt+del several times as kernel booting (before user level bootin), the kernel will oops. I found the ps2_command is called more than once, then the ps2dev->serio maybe NULL pointer. > > > > > > > > > > 2.6.22-rc5 and 2.6.22-rc6 have same result. > > > > > > > > > > Signed-off-by: Dave Young <hidave.darkstar@gmail.com> > > > > > --- > > > > > diff -upr linux/drivers/input/keyboard/atkbd.c linux.new/drivers/input/keyboard/atkbd.c > > > > > --- linux/drivers/input/keyboard/atkbd.c 2007-06-27 10:38:37.000000000 +0000 > > > > > +++ linux.new/drivers/input/keyboard/atkbd.c 2007-06-27 10:37:39.000000000 +0000 > > > > > @@ -795,6 +795,11 @@ static int atkbd_activate(struct atkbd * > > > > > > > > > > static void atkbd_cleanup(struct serio *serio) > > > > > { > > > > > + static int flag; > > > > > + > > > > > + if(flag) > > > > > + return; > > > > > + flag = 1; > > > > > > > > Unfortunately this will prevent atkbd from resetting keyboard on 2nd > > > > suspend attempt. It will also not work if you have an active MUX and > > > > have a couple of keyboards connected. > > > > > > > > Greg, now that you removed rwsem from subsystem (and subsystem itself > > > > for that matter) there is nothing as far as I can see that stops > > > > several threads from running device_shutdown() simultaneously. I also > > > > do not see what would isolate device probing and shutting them down > > > > at the same time. Am I missing something? > > > > > > There was never anything stopping that from happening before. No driver > > > core code was using that rwsem, so it wasn't protecting anything, > > > despite people trying to use it as if it was :) > > > > > > > It did protect device_shutdown() from itself, didn't it? > > Hm, yeah, it did, but that was it. If that was its goal, it sure wasn't > obvious at all. > > Do you think the driver core needs to serialize this? > I think that we need to have device tree (or rather list) stable while we performing system-wide state transitions, such as shutdown. Otheriwse if device_shutdown runs simultaneously with device discovery and new devices get added to the list we risk leaving them (and potentially thier parents) running. I think we had something like that before klist conversion - kobject_add was taking device_subsys.rwsem. However there are dangers if some device goes offline on its own right in the middle of shutdown process and bus code deadlocks... Maybe we only need to disable adding new devices in suspend/shutdown path while allowing removal... I am not sure. Overall there is lack of consistent locking in suspend/shutdown area: shutdown interface uses BKL, suspend (swsusp et all) uses pm_mutex and C_A_D handler does not have any locking at all at the moment. > thanks, > > greg k-h > -- Dmitry ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-06-28 6:13 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-06-27 10:59 [PATCH] atkbd: cleanup only once Dave Young 2007-06-27 3:01 ` dave young 2007-06-27 4:34 ` Dmitry Torokhov 2007-06-27 4:28 ` Greg KH 2007-06-27 4:59 ` Dmitry Torokhov 2007-06-27 5:02 ` dave young 2007-06-28 5:18 ` Dmitry Torokhov 2007-06-28 6:12 ` dave young 2007-06-27 5:27 ` Greg KH 2007-06-27 13:37 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).
![[-- Attachment #2: screen.jpg --] [-- Type: image/jpeg, Size: 117541 bytes --]](../../a8e1da0706262001w707f8beah72ed9e0d3e032a9d@mail.gmail.com/2-screen.jpg){kind=link}