On 2018-06-12 12:43:01 [-0400], Alan Stern wrote:
> On Tue, 12 Jun 2018, Sebastian Andrzej Siewior wrote:
> 
> > In the code path
> >   __usb_hcd_giveback_urb()
> >   -> wdm_in_callback()
> >    -> service_outstanding_interrupt()
> > 
> > The function service_outstanding_interrupt() will unconditionally enable
> > interrupts during unlock and invoke usb_submit_urb() with GFP_KERNEL.
> > If the HCD completes in BH (like ehci does) then the context remains
> > atomic due local_bh_disable() and enabling interrupts does not change
> > this.
> > 
> > Add an argument to service_outstanding_interrupt() which decides
> > whether or not it is save to enable interrupts during unlocking and use
> > GFP_KERNEL or not.
> 
> Wouldn't it be easier just to use spin_lock_irqsave() and GFP_ATOMIC
> all the time?  That's what people normally do with code that can be 
> called in both process and interrupt contexts.

service_outstanding_interrupt() does unlock + lock instead lock +
unlock. If you want to have this "always" working (without the
argument), we could do the false case:
+               spin_unlock(&desc->iuspin);
+               rv = usb_submit_urb(desc->response, GFP_ATOMIC);
+               spin_lock(&desc->iuspin);

all the time. I wanted to preserve the GFP_KERNEL part which is probably
used more often but fell that this is not needed I could drop that part.

> Alan Stern
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to