Re: [BUG] usbip: vhci: Sleeping function called from invalid context in vhci_urb_enqueue on PREEMPT_RT

Yunseong Kim
2025-08-16T02:18:10Z

Hi Alan,

On 8/16/25 10:51 AM, Alan Stern wrote:
> On Sat, Aug 16, 2025 at 10:29:34AM +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 USB/IP VHCI driver.
>>
>> On PREEMPT_RT configurations, standard spin_lock() calls are replaced by
>> rt_spin_lock(). Since rt_spin_lock() may sleep when contended, it must not
>> be called from an atomic context (e.g., with interrupts disabled).
>>
>> The issue occurs within the vhci_urb_enqueue function This function
>> explicitly disables local interrupts using local_irq_disable() immediately
>> before calling usb_hcd_giveback_urb(), adhering to HCD requirements.
>
> ...
>
>> This error reported after this work:
>> It occurs after going through the code below:
>>
>> static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
>> {
>>
>> ...
>>
>> no_need_unlink:
>> spin_unlock_irqrestore(&vhci->lock, flags);
>> if (!ret) {
>> /* usb_hcd_giveback_urb() should be called with
>> * irqs disabled
>> */
>> local_irq_disable(); // <--- Entering atomic context (IRQs disabled)
>> usb_hcd_giveback_urb(hcd, urb, urb->status);
>> local_irq_enable();
>> }
>> return ret;
>> }
>>
>> static void mon_bus_complete(struct mon_bus *mbus, struct urb *urb, int status)
>> {
>> ...
>> spin_lock_irqsave(&mbus->lock, flags);
> ^
> ------------------^
>
>> ...
>> }
>>
>> When called with interrupts disabled, usb_hcd_giveback_urb() eventually
>> leads to mon_complete() in the USB monitoring, if usbmon is enabled,
>> via __usb_hcd_giveback_urb().
>>
>> mon_complete() attempts to acquire a lock via spin_lock(), observed in the
>> trace within the inlined mon_bus_complete.
>
> Look again. mon_bus_complete() calls spin_lock_irqsave(), not
> spin_lock().
>
> Is the kernel tree that you are using different from Linus's tree?
I think this part is a macro, so it appears this way.

Link: https://github.com/torvalds/linux/blob/v6.17-rc1/include/linux/spinlock_rt.h#L96

#define spin_lock_irqsave(lock, flags) \
do { \
typecheck(unsigned long, flags); \
flags = 0; \
spin_lock(lock); \
} while (0)

My tree is indeed 6.17-rc1. I made a mistake in the diagram,
which caused the misunderstanding. I’ve redrawn the diagram:

kworker (hub_event)
|
v
vhci_urb_enqueue() [drivers/usb/usbip/vhci_hcd.c]
|
|---> spin_unlock_irqrestore(&vhci->lock, flags);
| (Context: IRQs Enabled, Process Context)
|---> local_irq_disable();
|
| *** STATE CHANGE: IRQs Disabled (Atomic Context) ***
|
+-----> usb_hcd_giveback_urb() [drivers/usb/core/hcd.c]
|
v
__usb_hcd_giveback_urb()
|
v
mon_complete() [drivers/usb/mon/mon_main.c]
|
|---> spin_lock_irqsave() [include/linux/spinlock_rt.h]
|
v https://github.com/torvalds/linux/blob/v6.17-rc1/include/linux/spinlock_rt.h#L96
spin_lock() [kernel/locking/spinlock_rt.c] <--- Attempts to acquire lock
|
| [On PREEMPT_RT]
v
rt_spin_lock() [kernel/locking/spinlock_rt.c]
|
v
[May Sleep if contended]
|
X <----------- BUG: Sleeping in atomic context (IRQs are disabled!)

|
|---> local_irq_enable();
(Context: IRQs Enabled)

> Alan Stern

Thank you!

Yunseong Kim