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

Alan Stern
2025-08-20T20:53:26Z

On Wed, Aug 20, 2025 at 06:26:21PM +0200, Sebastian Andrzej Siewior wrote:
> The problem is usually that nobody knows why exactly interrupts are
> disabled an what purpose it serves. Often the reasons is no longer there
> but the code still does it.

Indeed, that seems to be the problem in several places here.

> > More to the point, out of all the possible reasons why $SOMETHING might
> > be invoked with disabled interrupts, which of these reasons remain valid
> > in RT builds and which don't?
>
> None (in most cases) because interrupt handler are threaded. So
> interrupts are never truly disabled.
> Adding the macros as you suggest would gain probably three users:
> - inode
> - dummy_hcd
> - vhci-hcd
>
> Instead I would:
> - vhci I would suggest to remove the disabling and move its completion
> to BH.

Agreed. But it's up to the maintainer.

> - dummy_hcd I would suggest to either do the thing you called silly or
> audit the gadgets and remove it.

Auditing is the best approach. It would be a lot of work, though.
Also, it's worth noting that other UDC drivers do use the

spin_lock_irqsave();
...
spin_unlock();
usb_gadget_giveback_request();
spin_lock();
...
spin_unlock_irqrestore();

scheme, so using it in dummy_hcd would be reasonable.

> - inode I would suggest to keep it as-is and audit it properly later
> once someone intends to use it. It would also give the opportunity to
> clean up the commented locking statement.

I don't know if anyone is using AIO with gadgetfs any more. I've never
seen any applications for it other than to test the implementation.

Alan Stern