public inbox for linux-rt-users@vger.kernel.org
 help / color / mirror / Atom feed
* Spinlock in f_midi_transmit causing kernel crash
@ 2025-01-15  4:05 Jillian Donahue
  2025-01-23 13:23 ` Petr Tesařík
  0 siblings, 1 reply; 12+ messages in thread
From: Jillian Donahue @ 2025-01-15  4:05 UTC (permalink / raw)
  To: linux-rt-users

Hello,

I am working on getting USB MIDI enabled for my device and keep
running into this issue:

[  125.702601] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!
[  125.702607] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP

[  125.702764] Call trace:
[  125.702770]  rt_spin_lock_slowlock_locked+0x280/0x2b8
[  125.702774]  rt_spin_lock_slowlock+0x5c/0x90
[  125.702779]  rt_spin_lock+0x60/0x70
[  125.702792]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
[  125.702797]  f_midi_complete+0x11c/0x138 [usb_f_midi]
[  125.702809]  usb_gadget_giveback_request+0x10/0x18
[  125.702814]  __usbhsg_queue_pop+0x3c/0x58
[  125.702818]  usbhsg_queue_done+0x44/0x60
[  125.702822]  usbhsf_pkt_handler+0x80/0x120
[  125.702826]  usbhs_pkt_start+0x10/0x18
[  125.702830]  usbhsg_queue_push.isra.0+0x44/0x50
[  125.702834]  usbhsg_ep_queue+0x2c/0x40
[  125.702837]  usb_ep_queue+0x24/0x40
[  125.702843]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
[  125.702848]  f_midi_in_tasklet+0xc/0x18 [usb_f_midi]
[  125.702860]  tasklet_action_common.isra.2+0xcc/0x1f0
[  125.702864]  tasklet_hi_action+0x20/0x28
[  125.702869]  do_current_softirqs+0x19c/0x248
[  125.702873]  run_ksoftirqd+0x24/0x40
[  125.702883]  smpboot_thread_fn+0x1b0/0x2b8
[  125.702887]  kthread+0x16c/0x170
[  125.702894]  ret_from_fork+0x10/0x1c
[  125.702900] Code: a90363b7 f9000fb4 f90023b9 f9002bbb (d4210000)
[  125.966631] ---[ end trace 0000000000000002 ]---

Seen in the driver code here:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/usb/gadget/function/f_midi.c?h=v4.19.182#n683

We are running a 4.19.182 kernel patched to PREEMPT RT.

Has anyone come across this before?
I also ask if we've uncovered a bug when using the midi usb gadget
with PREEMPT RT.
If so, do you have any suggestions for a proper fix?

Thanks for your help,
~Jill Donahue

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-15  4:05 Spinlock in f_midi_transmit causing kernel crash Jillian Donahue
@ 2025-01-23 13:23 ` Petr Tesařík
  2025-01-24 22:21   ` Jillian Donahue
  0 siblings, 1 reply; 12+ messages in thread
From: Petr Tesařík @ 2025-01-23 13:23 UTC (permalink / raw)
  To: Jillian Donahue; +Cc: linux-rt-users

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

V Tue, 14 Jan 2025 23:05:10 -0500
Jillian Donahue <jilliandonahue58@gmail.com> napsáno:

> Hello,
> 
> I am working on getting USB MIDI enabled for my device and keep
> running into this issue:
> 
> [  125.702601] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!
> [  125.702607] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> 
> [  125.702764] Call trace:
> [  125.702770]  rt_spin_lock_slowlock_locked+0x280/0x2b8
> [  125.702774]  rt_spin_lock_slowlock+0x5c/0x90
> [  125.702779]  rt_spin_lock+0x60/0x70
> [  125.702792]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
> [  125.702797]  f_midi_complete+0x11c/0x138 [usb_f_midi]
> [  125.702809]  usb_gadget_giveback_request+0x10/0x18
> [  125.702814]  __usbhsg_queue_pop+0x3c/0x58
> [  125.702818]  usbhsg_queue_done+0x44/0x60
> [  125.702822]  usbhsf_pkt_handler+0x80/0x120
> [  125.702826]  usbhs_pkt_start+0x10/0x18
> [  125.702830]  usbhsg_queue_push.isra.0+0x44/0x50
> [  125.702834]  usbhsg_ep_queue+0x2c/0x40
> [  125.702837]  usb_ep_queue+0x24/0x40
> [  125.702843]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
> [  125.702848]  f_midi_in_tasklet+0xc/0x18 [usb_f_midi]
> [  125.702860]  tasklet_action_common.isra.2+0xcc/0x1f0
> [  125.702864]  tasklet_hi_action+0x20/0x28
> [  125.702869]  do_current_softirqs+0x19c/0x248
> [  125.702873]  run_ksoftirqd+0x24/0x40
> [  125.702883]  smpboot_thread_fn+0x1b0/0x2b8
> [  125.702887]  kthread+0x16c/0x170
> [  125.702894]  ret_from_fork+0x10/0x1c
> [  125.702900] Code: a90363b7 f9000fb4 f90023b9 f9002bbb (d4210000)
> [  125.966631] ---[ end trace 0000000000000002 ]---
> 
> Seen in the driver code here:
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/usb/gadget/function/f_midi.c?h=v4.19.182#n683
> 
> We are running a 4.19.182 kernel patched to PREEMPT RT.
> 
> Has anyone come across this before?
> I also ask if we've uncovered a bug when using the midi usb gadget
> with PREEMPT RT.
> If so, do you have any suggestions for a proper fix?

AFAICT the proper fix would be to replace tasklets with a work (as done
in commit c7d9efdff68e). IOW either upgrade to v5.10rt, or backport the
referenced commit.

HTH
Petr T

[-- Attachment #2: Digitální podpis OpenPGP --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-23 13:23 ` Petr Tesařík
@ 2025-01-24 22:21   ` Jillian Donahue
  2025-01-29 10:39     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Jillian Donahue @ 2025-01-24 22:21 UTC (permalink / raw)
  To: Petr Tesařík; +Cc: linux-rt-users

Thanks for your suggestion,

I should mention that I only see the spinlock on the midi transmit
side, not when receiving.
I have tried the proposed fixes and cherry-picked commits c7d9efdff68e
and 8653d71ce376 (the equivalent fix in f_midi.c), and upgraded to
v5.10rt. I still see the spin lock in both cases but the trace is
slightly different:

[  193.010603] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!
[  193.010609] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[  193.010644] Process kworker/4:2H (pid: 847, stack limit = 0x000000006e538c8b)
[  193.010666] Workqueue: events_highpri f_midi_in_work [usb_f_midi]
[  193.010671] pstate: 60000085 (nZCv daIf -PAN -UAO)
[  193.010686] pc : rt_spin_lock_slowlock_locked+0x280/0x2b8
[  193.010690] lr : rt_spin_lock_slowlock_locked+0x34/0x2b8
[  193.010691] sp : ffff00000c273a60
[  193.010693] x29: ffff00000c273a60 x28: ffff80053573c000
[  193.010696] x27: ffff80053573c220 x26: 0000000000000000
[  193.010699] x25: ffff80053575bc00 x24: 0000000000000000
[  193.010702] x23: ffff80053573c1d0 x22: ffff00000c273af8
[  193.010705] x21: ffff800537ced400 x20: ffff00000c273af8
[  193.010707] x19: ffff80053573c1e8 x18: 0000000000000000
[  193.010709] x17: 0000000000000000 x16: 0000000000000000
[  193.010711] x15: 0000000000000400 x14: 0000000000000400
[  193.010713] x13: 0000000000000001 x12: 0000000000000000
[  193.010715] x11: 00000000000001a4 x10: 0000000000000a10
[  193.010718] x9 : ffff00000c273d60 x8 : 0000000000000000
[  193.010720] x7 : ffff800537ced400 x6 : ffff800537ced401
[  193.010722] x5 : ffff80053573c200 x4 : ffff800537ced401
[  193.010724] x3 : 0000000000000001 x2 : 0000000000000000
[  193.010726] x1 : ffff800537ced400 x0 : ffff800537ced400
[  193.010729] Call trace:
[  193.010733]  rt_spin_lock_slowlock_locked+0x280/0x2b8
[  193.010736]  rt_spin_lock_slowlock+0x5c/0x90
[  193.010739]  rt_spin_lock+0x60/0x70
[  193.010742]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
[  193.010745]  f_midi_complete+0x11c/0x138 [usb_f_midi]
[  193.010754]  usb_gadget_giveback_request+0x10/0x18
[  193.010758]  __usbhsg_queue_pop+0x3c/0x58
[  193.010760]  usbhsg_queue_done+0x44/0x60
[  193.010763]  usbhsf_pkt_handler+0x80/0x120
[  193.010766]  usbhs_pkt_start+0x10/0x18
[  193.010768]  usbhsg_queue_push.isra.0+0x44/0x50
[  193.010771]  usbhsg_ep_queue+0x2c/0x40
[  193.010773]  usb_ep_queue+0x24/0x40
[  193.010776]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
[  193.010779]  f_midi_in_work+0x10/0x18 [usb_f_midi]
[  193.010786]  process_one_work+0x1e8/0x340
[  193.010788]  worker_thread+0x40/0x458
[  193.010792]  kthread+0x16c/0x170
[  193.010797]  ret_from_fork+0x10/0x1c
[  193.010801] Code: a90363b7 f9000fb4 f90023b9 f9002bbb (d4210000)
[  193.271972] ---[ end trace 0000000000000002 ]---
[  193.271976] note: kworker/4:2H[847] exited with preempt_count 1

Let me know if there is anything more useful here or if there's any
other information I can provide.

Thanks

On Thu, Jan 23, 2025 at 6:23 AM Petr Tesařík <petesarik@suse.de> wrote:
>
> V Tue, 14 Jan 2025 23:05:10 -0500
> Jillian Donahue <jilliandonahue58@gmail.com> napsáno:
>
> > Hello,
> >
> > I am working on getting USB MIDI enabled for my device and keep
> > running into this issue:
> >
> > [  125.702601] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!
> > [  125.702607] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> >
> > [  125.702764] Call trace:
> > [  125.702770]  rt_spin_lock_slowlock_locked+0x280/0x2b8
> > [  125.702774]  rt_spin_lock_slowlock+0x5c/0x90
> > [  125.702779]  rt_spin_lock+0x60/0x70
> > [  125.702792]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
> > [  125.702797]  f_midi_complete+0x11c/0x138 [usb_f_midi]
> > [  125.702809]  usb_gadget_giveback_request+0x10/0x18
> > [  125.702814]  __usbhsg_queue_pop+0x3c/0x58
> > [  125.702818]  usbhsg_queue_done+0x44/0x60
> > [  125.702822]  usbhsf_pkt_handler+0x80/0x120
> > [  125.702826]  usbhs_pkt_start+0x10/0x18
> > [  125.702830]  usbhsg_queue_push.isra.0+0x44/0x50
> > [  125.702834]  usbhsg_ep_queue+0x2c/0x40
> > [  125.702837]  usb_ep_queue+0x24/0x40
> > [  125.702843]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
> > [  125.702848]  f_midi_in_tasklet+0xc/0x18 [usb_f_midi]
> > [  125.702860]  tasklet_action_common.isra.2+0xcc/0x1f0
> > [  125.702864]  tasklet_hi_action+0x20/0x28
> > [  125.702869]  do_current_softirqs+0x19c/0x248
> > [  125.702873]  run_ksoftirqd+0x24/0x40
> > [  125.702883]  smpboot_thread_fn+0x1b0/0x2b8
> > [  125.702887]  kthread+0x16c/0x170
> > [  125.702894]  ret_from_fork+0x10/0x1c
> > [  125.702900] Code: a90363b7 f9000fb4 f90023b9 f9002bbb (d4210000)
> > [  125.966631] ---[ end trace 0000000000000002 ]---
> >
> > Seen in the driver code here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/usb/gadget/function/f_midi.c?h=v4.19.182#n683
> >
> > We are running a 4.19.182 kernel patched to PREEMPT RT.
> >
> > Has anyone come across this before?
> > I also ask if we've uncovered a bug when using the midi usb gadget
> > with PREEMPT RT.
> > If so, do you have any suggestions for a proper fix?
>
> AFAICT the proper fix would be to replace tasklets with a work (as done
> in commit c7d9efdff68e). IOW either upgrade to v5.10rt, or backport the
> referenced commit.
>
> HTH
> Petr T

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-24 22:21   ` Jillian Donahue
@ 2025-01-29 10:39     ` Sebastian Andrzej Siewior
  2025-01-29 21:59       ` Jillian Donahue
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-01-29 10:39 UTC (permalink / raw)
  To: Jillian Donahue; +Cc: Petr Tesařík, linux-rt-users

On 2025-01-24 15:21:21 [-0700], Jillian Donahue wrote:
…
> I have tried the proposed fixes and cherry-picked commits c7d9efdff68e
> and 8653d71ce376 (the equivalent fix in f_midi.c), and upgraded to
> v5.10rt. I still see the spin lock in both cases but the trace is
> slightly different:
> 
> [  193.010603] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!

This means the caller already owns the lock it asked for.

> [  193.010609] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> [  193.010644] Process kworker/4:2H (pid: 847, stack limit = 0x000000006e538c8b)
> [  193.010729] Call trace:
> [  193.010742]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
The lock it asks for

> [  193.010745]  f_midi_complete+0x11c/0x138 [usb_f_midi]
> [  193.010754]  usb_gadget_giveback_request+0x10/0x18
> [  193.010758]  __usbhsg_queue_pop+0x3c/0x58
> [  193.010760]  usbhsg_queue_done+0x44/0x60
> [  193.010763]  usbhsf_pkt_handler+0x80/0x120
> [  193.010766]  usbhs_pkt_start+0x10/0x18
> [  193.010768]  usbhsg_queue_push.isra.0+0x44/0x50
> [  193.010771]  usbhsg_ep_queue+0x2c/0x40
> [  193.010773]  usb_ep_queue+0x24/0x40
> [  193.010776]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
The lock it already acquired.

> [  193.010779]  f_midi_in_work+0x10/0x18 [usb_f_midi]
> [  193.010786]  process_one_work+0x1e8/0x340
> [  193.010788]  worker_thread+0x40/0x458
> [  193.010792]  kthread+0x16c/0x170
> [  193.010797]  ret_from_fork+0x10/0x1c
> 
> Let me know if there is anything more useful here or if there's any
> other information I can provide.

Looking at v6.13, the code flow is still the same. I don't see how !RT
is not affected by this. The tasklet to workqueue change didn't change
the logic here, it only changed the API and the context but the
recursion remains.

So a simple "tasklet_hi_schedule(&midi->tasklet);" instead of
"f_midi_transmit(midi)" in f_midi_complete() might do the trick.

> Thanks

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-29 10:39     ` Sebastian Andrzej Siewior
@ 2025-01-29 21:59       ` Jillian Donahue
  2025-01-30  7:50         ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Jillian Donahue @ 2025-01-29 21:59 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Petr Tesařík, linux-rt-users

> 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.

On Wed, Jan 29, 2025 at 3:39 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2025-01-24 15:21:21 [-0700], Jillian Donahue wrote:
> …
> > I have tried the proposed fixes and cherry-picked commits c7d9efdff68e
> > and 8653d71ce376 (the equivalent fix in f_midi.c), and upgraded to
> > v5.10rt. I still see the spin lock in both cases but the trace is
> > slightly different:
> >
> > [  193.010603] kernel BUG at /kernel-source//kernel/locking/rtmutex.c:1048!
>
> This means the caller already owns the lock it asked for.
>
> > [  193.010609] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > [  193.010644] Process kworker/4:2H (pid: 847, stack limit = 0x000000006e538c8b)
> …
> > [  193.010729] Call trace:
> …
> > [  193.010742]  f_midi_transmit+0x90/0x5d8 [usb_f_midi]
> The lock it asks for
>
> > [  193.010745]  f_midi_complete+0x11c/0x138 [usb_f_midi]
> > [  193.010754]  usb_gadget_giveback_request+0x10/0x18
> > [  193.010758]  __usbhsg_queue_pop+0x3c/0x58
> > [  193.010760]  usbhsg_queue_done+0x44/0x60
> > [  193.010763]  usbhsf_pkt_handler+0x80/0x120
> > [  193.010766]  usbhs_pkt_start+0x10/0x18
> > [  193.010768]  usbhsg_queue_push.isra.0+0x44/0x50
> > [  193.010771]  usbhsg_ep_queue+0x2c/0x40
> > [  193.010773]  usb_ep_queue+0x24/0x40
> > [  193.010776]  f_midi_transmit+0x238/0x5d8 [usb_f_midi]
> The lock it already acquired.
>
> > [  193.010779]  f_midi_in_work+0x10/0x18 [usb_f_midi]
> > [  193.010786]  process_one_work+0x1e8/0x340
> > [  193.010788]  worker_thread+0x40/0x458
> > [  193.010792]  kthread+0x16c/0x170
> > [  193.010797]  ret_from_fork+0x10/0x1c
> …
> >
> > Let me know if there is anything more useful here or if there's any
> > other information I can provide.
>
> Looking at v6.13, the code flow is still the same. I don't see how !RT
> is not affected by this. The tasklet to workqueue change didn't change
> the logic here, it only changed the API and the context but the
> recursion remains.
>
> So a simple "tasklet_hi_schedule(&midi->tasklet);" instead of
> "f_midi_transmit(midi)" in f_midi_complete() might do the trick.
>
> > Thanks
>
> Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-29 21:59       ` Jillian Donahue
@ 2025-01-30  7:50         ` Sebastian Andrzej Siewior
  2025-01-31 19:08           ` Jillian Donahue
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-01-30  7:50 UTC (permalink / raw)
  To: Jillian Donahue; +Cc: Petr Tesařík, linux-rt-users

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-30  7:50         ` Sebastian Andrzej Siewior
@ 2025-01-31 19:08           ` Jillian Donahue
  2025-02-03 12:45             ` Luis Claudio R. Goncalves
  0 siblings, 1 reply; 12+ messages in thread
From: Jillian Donahue @ 2025-01-31 19:08 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior; +Cc: Petr Tesařík, linux-rt-users

Thanks again for your time. Here is the request patch:

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-01-31 19:08           ` Jillian Donahue
@ 2025-02-03 12:45             ` Luis Claudio R. Goncalves
  2025-02-03 13:22               ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Luis Claudio R. Goncalves @ 2025-02-03 12:45 UTC (permalink / raw)
  To: Jillian Donahue
  Cc: Sebastian Andrzej Siewior, Petr Tesařík, linux-rt-users

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-02-03 12:45             ` Luis Claudio R. Goncalves
@ 2025-02-03 13:22               ` Sebastian Andrzej Siewior
  2025-02-03 17:50                 ` Jillian Donahue
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-02-03 13:22 UTC (permalink / raw)
  To: Luis Claudio R. Goncalves
  Cc: Jillian Donahue, Petr Tesařík, linux-rt-users

On 2025-02-03 09:45:24 [-0300], Luis Claudio R. Goncalves wrote:
> 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.

Jillian, could you please follow
	https://docs.kernel.org/process/submitting-patches.html

The Fixes tag should be
   Fixes:  d5daf49b58661 ("USB: gadget: midi: add midi function driver")

I suggest that this is posted upstream and applied there, backported as
a fix via the official stable tree. As I mentioned this is not
PREEMPT_RT specific.

> Thanks in advance!
> Luis

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-02-03 13:22               ` Sebastian Andrzej Siewior
@ 2025-02-03 17:50                 ` Jillian Donahue
  2025-02-06 12:05                   ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Jillian Donahue @ 2025-02-03 17:50 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Luis Claudio R. Goncalves, Petr Tesařík, linux-rt-users

From a97fceb3e51c478dee6d63caafb92c867432c042 Mon Sep 17 00:00:00 2001
From: Jill Donahue <jilliandonahue58@gmail.com>
Date: Mon, 3 Feb 2025 10:46:31 -0700
Subject: [PATCH] f_midi_complete to call tasklet_hi_schedule

When using USB MIDI, a spinlock is hit causing a kernel oops. This
happens because a lock is attempted to be acquired twice through
f_midi_transmit. The fix lies in calling tasklet_hi_schedule() in
f_midi_complete to remove the repeated call to f_midi_transmit.

Link: https://lore.kernel.org/all/CAArt=LjxU0fUZOj06X+5tkeGT+6RbXzpWg1h4t4Fwa_KGVAX6g@mail.gmail.com/

Fixes: d5daf49b58661 ("USB: gadget: midi: add midi function driver")
---
 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-02-03 17:50                 ` Jillian Donahue
@ 2025-02-06 12:05                   ` Sebastian Andrzej Siewior
  2025-02-06 18:13                     ` Jillian Donahue
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2025-02-06 12:05 UTC (permalink / raw)
  To: Jillian Donahue
  Cc: Luis Claudio R. Goncalves, Petr Tesařík, linux-rt-users

On 2025-02-03 10:50:57 [-0700], Jillian Donahue wrote:
> From a97fceb3e51c478dee6d63caafb92c867432c042 Mon Sep 17 00:00:00 2001
> From: Jill Donahue <jilliandonahue58@gmail.com>
> Date: Mon, 3 Feb 2025 10:46:31 -0700
> Subject: [PATCH] f_midi_complete to call tasklet_hi_schedule
> 
> When using USB MIDI, a spinlock is hit causing a kernel oops. This

It is not an oops, it is a deadlock and in general a lock-up. On
PREEMPT_RT you see this backtrace. Non-RT with lockdep should report
something. I would stay with deadlock.

> happens because a lock is attempted to be acquired twice through
> f_midi_transmit. The fix lies in calling tasklet_hi_schedule() in
> f_midi_complete to remove the repeated call to f_midi_transmit.

s/The.*

Use tasklet_hi_schedule() to schedule f_midi_transmit() via a tasklet
from the completion handler.

> Link: https://lore.kernel.org/all/CAArt=LjxU0fUZOj06X+5tkeGT+6RbXzpWg1h4t4Fwa_KGVAX6g@mail.gmail.com/
> 
> Fixes: d5daf49b58661 ("USB: gadget: midi: add midi function driver")

Way better. Please run it via ./scripts/check_patch.pl. It will tell you
that you need a sign-of-by line. Please check what it means and if you
agree with it.

Please run it via scripts/get_maintainer.pl and send it to that list.

Thank you.

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Spinlock in f_midi_transmit causing kernel crash
  2025-02-06 12:05                   ` Sebastian Andrzej Siewior
@ 2025-02-06 18:13                     ` Jillian Donahue
  0 siblings, 0 replies; 12+ messages in thread
From: Jillian Donahue @ 2025-02-06 18:13 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: Luis Claudio R. Goncalves, Petr Tesařík, linux-rt-users

Thanks for your patience - this is my first time writing and
submitting a patch. Will send to the proper channels shortly.

Jill

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-02-06 18:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-15  4:05 Spinlock in f_midi_transmit causing kernel crash Jillian Donahue
2025-01-23 13:23 ` Petr Tesařík
2025-01-24 22:21   ` Jillian Donahue
2025-01-29 10:39     ` Sebastian Andrzej Siewior
2025-01-29 21:59       ` Jillian Donahue
2025-01-30  7:50         ` Sebastian Andrzej Siewior
2025-01-31 19:08           ` Jillian Donahue
2025-02-03 12:45             ` Luis Claudio R. Goncalves
2025-02-03 13:22               ` Sebastian Andrzej Siewior
2025-02-03 17:50                 ` Jillian Donahue
2025-02-06 12:05                   ` Sebastian Andrzej Siewior
2025-02-06 18:13                     ` Jillian Donahue

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox