Re: Spinlock in f_midi_transmit causing kernel crash

["Luis Claudio R. Goncalves" <lgoncalv@redhat.com>]

On Fri, Jan 31, 2025 at 12:08:01PM -0700, Jillian Donahue wrote:
> Thanks again for your time. Here is the request patch:

Sebastian, do you envision this as a patch to be carried by v5.10-rt or a
patch to stable v5.10?

In any case, Jillian, would you mind adding a description to your patch? No
matter who will take the patch (Greg or myself), a description is required.
One or two paragraphs summarizing the problem and/or the solution.

Thanks in advance!
Luis
 
> From 1577bf36766218a1902ac92377dbc51798f2e4a0 Mon Sep 17 00:00:00 2001
> From: Jill Donahue <jilliandonahue58@gmail.com>
> Date: Fri, 31 Jan 2025 11:39:57 -0700
> Subject: [PATCH] f_midi_complete to call tasklet_hi_schedule
> 
> ---
>  drivers/usb/gadget/function/f_midi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_midi.c
> b/drivers/usb/gadget/function/f_midi.c
> index 837fcdfa3..37d438e5d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -283,7 +283,7 @@ f_midi_complete(struct usb_ep *ep, struct usb_request *req)
>                         /* Our transmit completed. See if there's more to go.
>                          * f_midi_transmit eats req, don't queue it again. */
>                         req->length = 0;
> -                       f_midi_transmit(midi);
> +                       tasklet_hi_schedule(&midi->tasklet);
>                         return;
>                 }
>                 break;
> -- 
> 2.25.1
> 
> Jill
> 
> On Thu, Jan 30, 2025 at 12:50 AM Sebastian Andrzej Siewior
> <bigeasy@linutronix.de> wrote:
> >
> > On 2025-01-29 14:59:19 [-0700], Jillian Donahue wrote:
> > > > So a simple "tasklet_hi_schedule(&midi->tasklet);" instead of "f_midi_transmit(midi)" in f_midi_complete() might do the trick.
> > >
> > > This fixes the problem - do you have any insight to how this worked in
> > > the first place? Trying to understand the change.
> >
> > There are possibilities. The problem is that you send a packet and it
> > completes the same moment and the completion callback is invoked which
> > deadlocks.
> >
> > Now:
> > - PREEMPT_RT may have moved the timing a bit (makes it behave a bit
> >   different or simply make a UP device behave like a SMP one) to the
> >   point that it sees the completion of the request where it did not
> >   before. So does it occur without PREEMPT_RT?
> >
> > - It never did work in this combination even without PREEMPT_RT. The
> >   driver (f_midi) was never tested with the USB device controller you
> >   have and the one it was tested with behaves differently so the
> >   recursion never occurred.
> >
> > - It is an old driver (f_midi). It was never tested on SMP or with
> >   enabled lock testing. On UP spinlocks end up almost as NOPs so a
> >   deadlock (as in this case) will not be observed.
> >
> > Either way I would blame f_midi here.
> > Once you have the pieces together, mind sending a patch?
> >
> > Sebastian
> 
---end quoted text---