Re: [linux-usb-devel] Fw: X is killed when trying to suspend with USB Mouse plugged in

Re: [linux-usb-devel] Fw: X is killed when trying to suspend with USB Mouse plugged in

[Alan Stern]

On Mon, 18 Oct 2004, Andrew Morton wrote:

> Begin forwarded message:
> 
> Date: Mon, 18 Oct 2004 20:55:16 +0200
> From: Nils Rennebarth <Nils.Rennebarth@web.de>
> To: linux-kernel@vger.kernel.org
> Cc: Suspend development list <softwaresuspend-devel@lists.berlios.de>
> Subject: X is killed when trying to suspend with USB Mouse plugged in
> 
> 
> Hi,
> 
> When I try to suspend 2.6.9-rc[1-4] when X is runnning and my USB Mouse 
> is plugged in, I get an Ooops. X is killed and the suspend as well.
> 
> Attached is the oops with 2.6.9-rc4
> The oops comes at the moment, that uhci_hcd is removed. If I do not 
> remove that module, suspend does work but the laptop hangs hard during 
> resume.
> 
> That only happens if I use /dev/input/mouse1 as input device for the 
> mouse. With /dev/input/mice I can suspend and resume successfully.
> 
> So is using /dev/input/mouse1 something I shouldn't have done in the 
> first place (came from some experimentation when trying to use the 
> synaptics driver for my alps touchpad) or does it point to a bug in the 
> uhci driver? Or a bug in X?

I don't know about /dev/input/mouse1.  But the oops isn't a bug... it's a 
weakness in the way Linux implements loadable kernel modules.

What your log showed was that some program was still using a USB device at
the time uhci-hcd was unloaded.  There's nothing illegal or wrong about
that, but the unload procedure doesn't wait for the reference count to
drop to zero -- the module is unloaded from memory right away.  Then some
time later on, when that other program finished using the device and the
reference count did go to zero, the system tried to call a release routine
in the unloaded module.  The result was an oops, as you saw.

Alan Stern

Re: [linux-usb-devel] Fw: X is killed when trying to suspend with USB Mouse plugged in

[Dmitry Torokhov]

On Monday 18 October 2004 05:48 pm, Alan Stern wrote:
> On Mon, 18 Oct 2004, Andrew Morton wrote:
> 
> > Hi,
> > 
> > When I try to suspend 2.6.9-rc[1-4] when X is runnning and my USB Mouse 
> > is plugged in, I get an Ooops. X is killed and the suspend as well.
> > 
> > Attached is the oops with 2.6.9-rc4
> > The oops comes at the moment, that uhci_hcd is removed. If I do not 
> > remove that module, suspend does work but the laptop hangs hard during 
> > resume.
> > 
> > That only happens if I use /dev/input/mouse1 as input device for the 
> > mouse. With /dev/input/mice I can suspend and resume successfully.
> > 
> > So is using /dev/input/mouse1 something I shouldn't have done in the 
> > first place (came from some experimentation when trying to use the 
> > synaptics driver for my alps touchpad) or does it point to a bug in the 
> > uhci driver? Or a bug in X?
> 
> I don't know about /dev/input/mouse1.  But the oops isn't a bug... it's a 
> weakness in the way Linux implements loadable kernel modules.
> 

Ugh, it is not module implementation weakness, it looks like refcounting
problem in USB. Anyway, I wonder if the following patch will help (hide)
the problem (it is needed anyway so mousedv et all modules can be
unloaded at any time).

In any case I recommend using /dev/input/mice as it is always available,
otherwise your mouse can jump between various /dev/input/mouseX devices.

-- 
Dmitry


===================================================================


ChangeSet@1.1958, 2004-09-30 01:49:20-05:00, dtor_core@ameritech.net
  Input: evdev, joydev, mousedev, tsdev - remove class device and devfs
         entry when hardware driver disconnects instead of waiting for
         the last user to drop off. This way hardware drivers can be
         unloaded at any time.
  
  Signed-off-by: Dmitry Torokhov <dtor@mail.ru>


 evdev.c    |    4 ++--
 joydev.c   |    4 ++--
 mousedev.c |    4 ++--
 tsdev.c    |   10 +++++-----
 4 files changed, 11 insertions(+), 11 deletions(-)


===================================================================



diff -Nru a/drivers/input/evdev.c b/drivers/input/evdev.c
--- a/drivers/input/evdev.c	2004-10-09 23:55:27 -05:00
+++ b/drivers/input/evdev.c	2004-10-09 23:55:27 -05:00
@@ -91,8 +91,6 @@
 
 static void evdev_free(struct evdev *evdev)
 {
-	devfs_remove("input/event%d", evdev->minor);
-	class_simple_device_remove(MKDEV(INPUT_MAJOR, EVDEV_MINOR_BASE + evdev->minor));
 	evdev_table[evdev->minor] = NULL;
 	kfree(evdev);
 }
@@ -441,6 +439,8 @@
 {
 	struct evdev *evdev = handle->private;
 
+	class_simple_device_remove(MKDEV(INPUT_MAJOR, EVDEV_MINOR_BASE + evdev->minor));
+	devfs_remove("input/event%d", evdev->minor);
 	evdev->exist = 0;
 
 	if (evdev->open) {
diff -Nru a/drivers/input/joydev.c b/drivers/input/joydev.c
--- a/drivers/input/joydev.c	2004-10-09 23:55:27 -05:00
+++ b/drivers/input/joydev.c	2004-10-09 23:55:27 -05:00
@@ -143,9 +143,7 @@
 
 static void joydev_free(struct joydev *joydev)
 {
-	devfs_remove("input/js%d", joydev->minor);
 	joydev_table[joydev->minor] = NULL;
-	class_simple_device_remove(MKDEV(INPUT_MAJOR, JOYDEV_MINOR_BASE + joydev->minor));
 	kfree(joydev);
 }
 
@@ -466,6 +464,8 @@
 {
 	struct joydev *joydev = handle->private;
 
+	class_simple_device_remove(MKDEV(INPUT_MAJOR, JOYDEV_MINOR_BASE + joydev->minor));
+	devfs_remove("input/js%d", joydev->minor);
 	joydev->exist = 0;
 
 	if (joydev->open)
diff -Nru a/drivers/input/mousedev.c b/drivers/input/mousedev.c
--- a/drivers/input/mousedev.c	2004-10-09 23:55:27 -05:00
+++ b/drivers/input/mousedev.c	2004-10-09 23:55:27 -05:00
@@ -335,8 +335,6 @@
 
 static void mousedev_free(struct mousedev *mousedev)
 {
-	devfs_remove("input/mouse%d", mousedev->minor);
-	class_simple_device_remove(MKDEV(INPUT_MAJOR, MOUSEDEV_MINOR_BASE + mousedev->minor));
 	mousedev_table[mousedev->minor] = NULL;
 	kfree(mousedev);
 }
@@ -646,6 +644,8 @@
 {
 	struct mousedev *mousedev = handle->private;
 
+	class_simple_device_remove(MKDEV(INPUT_MAJOR, MOUSEDEV_MINOR_BASE + mousedev->minor));
+	devfs_remove("input/mouse%d", mousedev->minor);
 	mousedev->exist = 0;
 
 	if (mousedev->open) {
diff -Nru a/drivers/input/tsdev.c b/drivers/input/tsdev.c
--- a/drivers/input/tsdev.c	2004-10-09 23:55:27 -05:00
+++ b/drivers/input/tsdev.c	2004-10-09 23:55:27 -05:00
@@ -1,7 +1,7 @@
 /*
  * $Id: tsdev.c,v 1.15 2002/04/10 16:50:19 jsimmons Exp $
  *
- *  Copyright (c) 2001 "Crazy" james Simmons 
+ *  Copyright (c) 2001 "Crazy" james Simmons
  *
  *  Compaq touchscreen protocol driver. The protocol emulated by this driver
  *  is obsolete; for new programs use the tslib library which can read directly
@@ -177,8 +177,6 @@
 
 static void tsdev_free(struct tsdev *tsdev)
 {
-	devfs_remove("input/ts%d", tsdev->minor);
-	class_simple_device_remove(MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + tsdev->minor));
 	tsdev_table[tsdev->minor] = NULL;
 	kfree(tsdev);
 }
@@ -418,7 +416,7 @@
 			S_IFCHR|S_IRUGO|S_IWUSR, "input/ts%d", minor);
 	devfs_mk_cdev(MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + minor + TSDEV_MINORS/2),
 			S_IFCHR|S_IRUGO|S_IWUSR, "input/tsraw%d", minor);
-	class_simple_device_add(input_class, 
+	class_simple_device_add(input_class,
 				MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + minor),
 				dev->dev, "ts%d", minor);
 
@@ -429,6 +427,9 @@
 {
 	struct tsdev *tsdev = handle->private;
 
+	class_simple_device_remove(MKDEV(INPUT_MAJOR, TSDEV_MINOR_BASE + tsdev->minor));
+	devfs_remove("input/ts%d", tsdev->minor);
+	devfs_remove("input/tsraw%d", tsdev->minor);
 	tsdev->exist = 0;
 
 	if (tsdev->open) {
@@ -436,7 +437,6 @@
 		wake_up_interruptible(&tsdev->wait);
 	} else
 		tsdev_free(tsdev);
-	devfs_remove("input/tsraw%d", tsdev->minor);
 }
 
 static struct input_device_id tsdev_ids[] = {

Re: [linux-usb-devel] Fw: X is killed when trying to suspend with USB Mouse plugged in

[Alan Stern]

On Mon, 18 Oct 2004, Dmitry Torokhov wrote:

> > I don't know about /dev/input/mouse1.  But the oops isn't a bug... it's a 
> > weakness in the way Linux implements loadable kernel modules.
> > 
> 
> Ugh, it is not module implementation weakness, it looks like refcounting
> problem in USB.

Could you explain that more fully?  Are you talking about a particular 
refcounting problem in the usbhid subsystem or do you mean a more 
pervasive problem in the whole USB system?  And why do you say it's a 
refcounting problem in the first place?

Alan Stern

Re: [linux-usb-devel] Fw: X is killed when trying to suspend with USB Mouse plugged in

[Dmitry Torokhov]

On Tuesday 19 October 2004 10:35 am, Alan Stern wrote:
> On Mon, 18 Oct 2004, Dmitry Torokhov wrote:
> 
> > > I don't know about /dev/input/mouse1.  But the oops isn't a bug... it's a 
> > > weakness in the way Linux implements loadable kernel modules.
> > > 
> > 
> > Ugh, it is not module implementation weakness, it looks like refcounting
> > problem in USB.
> 
> Could you explain that more fully?  Are you talking about a particular 
> refcounting problem in the usbhid subsystem or do you mean a more 
> pervasive problem in the whole USB system?  And why do you say it's a 
> refcounting problem in the first place?
> 

I am not sure it it is HID-specific problem or a wider one but it looks
like usbhid can be unloaded while there are references to objects produced
by this module - hence refcounting problem. You either have to disallow
unloading while there are references (but this path leads to potential
deadlocks) or have a generic release function registered with the core that
pretty much always stays there. Then you can free all device-specific data
at unload time and mark the object as a zombie so anything that tries to
touch it releases it quickly and then the core routine will free skeleton
data at last.

The patch that I sent should hide the problem somewhat as at disconnect
time it will unregister corresponsing class devices thus dropping the
reference that was pinning usbhid structures.

-- 
Dmitry