usbmouse: random freeze/hangup; synchronize irq completion

Hello,

usbmouse: synchonize the irq completion for each urb.

Without this patch the mouse was a bit erratic and when
using the mouse the system gets freezed/hangup randomly (one or two times a day).

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

usbmouse: synchonize the irq completion

@@ -1214,6 +1214,7 @@ struct urb {
 	int error_count;		/* (return) number of ISO errors */
 	void *context;			/* (in) context for completion */
 	usb_complete_t complete;	/* (in) completion routine */
+	struct mutex		complete_mutex;		/* synchronize completion */
 	struct usb_iso_packet_descriptor iso_frame_desc[0];
 					/* (in) ISO ONLY */
 };

@@ -1250,6 +1251,7 @@ static inline void usb_fill_control_urb(
 	urb->transfer_buffer_length = buffer_length;
 	urb->complete = complete_fn;
 	urb->context = context;
+	mutex_init(&urb->complete_mutex);
 }
 
 /**

@@ -1279,6 +1281,7 @@ static inline void usb_fill_bulk_urb(str
 	urb->transfer_buffer_length = buffer_length;
 	urb->complete = complete_fn;
 	urb->context = context;
+	mutex_init(&urb->complete_mutex);
 }
 
 /**

@@ -1326,6 +1329,7 @@ static inline void usb_fill_int_urb(stru
 	else
 		urb->interval = interval;
 	urb->start_frame = -1;
+	mutex_init(&urb->complete_mutex);
 }
 
 extern void usb_init_urb(struct urb *urb);

@@ -66,13 +71,15 @@ static void usb_mouse_irq(struct urb *ur
 	struct input_dev *dev = mouse->dev;
 	int status;
 
+	mutex_lock(&urb->complete_mutex);
+
 	switch (urb->status) {
 	case 0:			/* success */
 		break;
 	case -ECONNRESET:	/* unlink */
 	case -ENOENT:
 	case -ESHUTDOWN:
-		return;
+		goto out;
 	/* -EPIPE:  should clear the halt */
 	default:		/* error */
 		goto resubmit;

@@ -95,6 +102,9 @@ resubmit:
 		err ("can't resubmit intr, %s-%s/input0, status %d",
 				mouse->usbdev->bus->bus_name,
 				mouse->usbdev->devpath, status);
+
+out:
+	mutex_unlock(&urb->complete_mutex);
 }
 
 static int usb_mouse_open(struct input_dev *dev)

Your patch is no good, for quite a few reasons. I'll state only two of them:

It calls mutex_lock(), which can sleep, from within an URB completion routine, which runs with interrupts disabled.

It doesn't synchronize the completion _with_ anything else, i.e., it calls mutex_lock() in only one place.

Whatever is causing your problem, this is not the proper solution.

Alan Stern

A Dilluns 14 Febrer 2011 16:00:24, Alan Stern va escriure:

this routine is synchronized with himself.
I said synchronize thinking about synchronized routines that are pieces of code scheduled to be executed only in a single thread simultaneously; therefore a new call to this routine can not be executed until a previous call has been completed.

Yes, that this is not a patch to be included in the kernel, it's only a testing patch, but it solves the hangup. and so this probes that there is some kind of problem related to the urb update process; I think the problem should be external to this routine.

searching the web, some people has found problems like this, example:
- [ubuntu] Ubuntu 10.04 (Lucid Lynx) Random Freeze / Hang-up

Consider this as an idea to point out the problem and, if possible, someone more expert help to look for a good solution.

Jordi Pujol

Live never ending Tale
GNU/Linux Live forever!
http://livenet.selfip.com

No. The code you added does not perform _any_ additional synchronization. If it did, you'd see a big fat error message in the kernel log because the mutex_lock call would try to sleep while in interrupt context.

That is true even without your patch. Go ahead and add a pair of debugging printouts to the usb_mouse_irq() routine (without your patch); print the value of urb upon entry and exit. You'll see that there are never two instances of the routine running comcurrently for the same URB. In fact, if you have only one USB mouse, you'll see that there are never two instances of the routine running concurrently at all.

More to the point, why are you using the usbmouse driver in the first place? You should be using usbhid instead. See the warnings in the Kconfig help text for USB_MOUSE.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html