On Sat, Aug 16, 2025 at 11:38:14AM +0900, Yunseong Kim wrote:
> While testing a PREEMPT_RT enabled kernel (based on v6.17.0-rc1),
> I encountered a "BUG: sleeping function called from invalid context" error
> originating from the dummy_dequeue function in the dummy USB driver.
...
> The pattern of manually disabling IRQs and then taking a spinlock
> local_irq_save() + spin_lock() is unsafe on PREEMPT_RT, the current code
> structure keeps IRQs disabled even after spin_unlock(&dum->lock) while
> calling usb_gadget_giveback_request(). This extended atomic context can
> also be problematic if the completion handler attempts to acquire another
> sleepable lock.
I don't know the USB subsystem well, but the comments above struct
usb_request says:
* @complete: Function called when request completes, so this request and
* its buffer may be re-used. The function will always be called with
* interrupts disabled, and it must not sleep.
Therefore it shouldn't be a concern that "completion handler attempts to
acquire another sleepable lock".
> I request a review and correction of this locking mechanism to ensure
> stability on PREEMPT_RT configurations. Kernel config, full logs, and
> reproduction steps can be provided on request.
This was introduced by b4dbda1a22d2 ("USB: dummy-hcd: disable interrupts
during req->complete") which split the spin_lock_irqsave() into
local_irq_save() and spin_lock().
The untested patch below should help?
Enabling interrupt (spin_unlock_irqrestore) and then immediately disabling
interrupt (local_irq_save) is not the nicest thing. But then I don't see
how to avoid that while being non-hacky and human-readable.
Nam
diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 21dbfb0b3bac..a4653c919664 100644
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -765,8 +765,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
if (!dum->driver)
return -ESHUTDOWN;
- local_irq_save(flags);
- spin_lock(&dum->lock);
+ spin_lock_irqsave(&dum->lock, flags);
list_for_each_entry(iter, &ep->queue, queue) {
if (&iter->req != _req)
continue;
@@ -776,15 +775,16 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req)
retval = 0;
break;
}
- spin_unlock(&dum->lock);
+ spin_unlock_irqrestore(&dum->lock, flags);
if (retval == 0) {
dev_dbg(udc_dev(dum),
"dequeued req %p from %s, len %d buf %p\n",
req, _ep->name, _req->length, _req->buf);
+ local_irq_save(flags);
usb_gadget_giveback_request(_ep, _req);
+ local_irq_restore(flags);
}
- local_irq_restore(flags);
return retval;
}
