Re: [BUG] usb: gadget: dummy_hcd: Sleeping function called from invalid context in dummy_dequeue on PREEMPT_RT

[Nam Cao <namcao@linutronix.de>]

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;
 }