On Tue, Apr 24, 2012 at 10:22 PM, Oliver Neukum [off-list ref] wrote:

Am Dienstag, 24. April 2012, 06:19:00 schrieb Ming Lei:

quoted

@@ -486,11 +494,15 @@ static void hid_ctrl(struct urb *urb)

      if (usbhid->ctrlhead != usbhid->ctrltail && !hid_submit_ctrl(hid)) {
              /* Successfully submitted next urb in queue */
+             if (status != -ECONNRESET)
+                     spin_unlock(&usbhid->unlink_lock);
              spin_unlock(&usbhid->lock);
              return;
      }

      clear_bit(HID_CTRL_RUNNING, &usbhid->iofl);
+     if (status != -ECONNRESET)
+             spin_unlock(&usbhid->unlink_lock);
      spin_unlock(&usbhid->lock);
      usb_autopm_put_interface_async(usbhid->intf);
      wake_up(&usbhid->wait);

Now you race against a double time out

CPU A                                                           CPU B

__usbhid_submit_report()
time_after()
usb_unlink_urb()
-- this has to go to the hardware -->
                                                                       hid_irq_out()
                                                                       if (status != -ECONNRESET)
                                                                       --> no lock
                                                                       hid_submit_out()
__usbhid_submit_report()
time_after()
                                                                       usb_submit_urb()

This submit won't happen because HID_OUT_RUNNING is not cleared.

usb_unlink_urb()

quoted

@@ -546,8 +558,13 @@ static void __usbhid_submit_report(struct

hid_device *hid, struct hid_report *re
                       * no race because this is called under
                       * spinlock
                       */
-                     if (time_after(jiffies, usbhid->last_out + HZ * 5))
+                     spin_lock(&usbhid->unlink_lock);
+                     if (time_after(jiffies, usbhid->last_out + HZ * 5)) {
+                             spin_unlock(&usbhid->lock);
                              usb_unlink_urb(usbhid->urbout);
+                             spin_lock(&usbhid->lock);
+                     }
+                     spin_unlock(&usbhid->unlink_lock);

AB-BA deadlock

OK, if we always acquire unlink_lock before lock in usbhid_submit_report,
hid_led, hid_ctrl, and hid_irq_out, the AB-BA deadlock can be removed.

Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html