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

[Sebastian Andrzej Siewior]

On 2025-08-16 08:59:33 [+0200], Nam Cao wrote:
> 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);

The two above are fine.

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

This is not. I don't see the need for it. The queue part does
	spin_lock_irqsave()
	spin_unlock();
	usb_gadget_giveback_request()
	spin_lock();
	spin_unlock_irqrestore();

and keeps the interrupts disabled during callback invocation. This seems
to be just to unify the code vs the else path.

>  	}
> -	local_irq_restore(flags);
>  	return retval;
>  }

Sebastian