Hi Geert-san,
> > Still, that would need some better protection, as local_irq_save() disables
> > interrupts only on the CPU it's running on, not on other CPUs in a
> > multiprocessor system.
>
> I see. I will investigate this issue more.
I tried this issue on v3.19 with dmac enabled, it also caused an oops.
However, the log is useful to investigate this issue.
This issue is caused by tx_complete() and ncm_tx_tasklet() because
the renesas_usbhs driver called usb_gadget_giveback_request with
interrupts enabled. So, spin_lock_irqsave(&dev->req_lock, flags) in
u_ether.c is held.
So, I will submit a patch v2 as the followings. What do you think?
--
Subject: [PATCH] usb: renesas_usbhs: fix spinlock suspected in a gadget complete function
According to the gadget.h, a "complete" function will always be called
with interrupts disabled. However, sometimes usbhsg_queue_pop() function
is called with interrupts enabled. So, this function should calls
local_irq_save() before this calls the usb_gadget_giveback_request().
Otherwise, there is possible to cause a spinlock suspected in a gadget
complete function.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
---
drivers/usb/renesas_usbhs/mod_gadget.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/renesas_usbhs/mod_gadget.c b/drivers/usb/renesas_usbhs/mod_gadget.c
index e0384af..104bddf 100644
--- a/drivers/usb/renesas_usbhs/mod_gadget.c
+++ b/drivers/usb/renesas_usbhs/mod_gadget.c
@@ -126,11 +126,22 @@ static void usbhsg_queue_pop(struct usbhsg_uep *uep,
struct usbhsg_gpriv *gpriv = usbhsg_uep_to_gpriv(uep);
struct usbhs_pipe *pipe = usbhsg_uep_to_pipe(uep);
struct device *dev = usbhsg_gpriv_to_dev(gpriv);
+ unsigned long flags;
dev_dbg(dev, "pipe %d : queue pop\n", usbhs_pipe_number(pipe));
ureq->req.status = status;
+ /*
+ * According to the gadget.h, a "complete" function will always be
+ * called with interrupts disabled. However, sometimes this function
+ * is called with interrupts enabled. (e.g. complete a DMAC transfer or
+ * write data and done is set immediately when PIO.) So, this function
+ * should calls local_irq_save() before this calls the
+ * usb_gadget_giveback_request().
+ */
+ local_irq_save(flags);
usb_gadget_giveback_request(&uep->ep, &ureq->req);
+ local_irq_restore(flags);
}
static void usbhsg_queue_done(struct usbhs_priv *priv, struct usbhs_pkt *pkt)
======= oops log on v3.19 + dmac =======
BUG: spinlock lockup suspected on CPU#0, irq/102-e65a000/547
lock: 0xeea2dd5c, .magic: dead4ead, .owner: <none>/-1, .owner_cpu: -1
CPU: 0 PID: 547 Comm: irq/102-e65a000 Tainted: G W 3.19.0-05634-g11371d7-dirty #55
Hardware name: Generic R8A7791 (Flattened Device Tree)
Backtrace:
[<c0011e78>] (dump_backtrace) from [<c0012084>] (show_stack+0x18/0x1c)
r6:00989680 r5:eea2dd5c r4:00000000 r3:00208040
[<c001206c>] (show_stack) from [<c0471338>] (dump_stack+0x7c/0x98)
[<c04712bc>] (dump_stack) from [<c0470630>] (spin_dump+0x80/0x94)
r4:00000000 r3:c0671334
[<c04705b0>] (spin_dump) from [<c0055910>] (do_raw_spin_lock+0x13c/0x190)
r5:00000000 r4:00989680
[<c00557d4>] (do_raw_spin_lock) from [<c04751c0>] (_raw_spin_lock_irqsave+0x18/0x20)
r9:eea2dd5c r8:eea2dd40 r7:eebaa034 r6:00000000 r5:0000000f r4:60000113
[<c04751a8>] (_raw_spin_lock_irqsave) from [<bf0006bc>] (eth_start_xmit+0xd0/0x37c [u_ether])
r4:eea2d800 r3:00000000
[<bf0005ec>] (eth_start_xmit [u_ether]) from [<bf01b0e8>] (ncm_tx_tasklet+0x44/0x4c [usb_f_ncm])
r10:eeb87d00 r9:c06b6b00 r8:00000000 r7:00000000 r6:c0671274 r5:00000000
r4:ee0f8e00
[<bf01b0a4>] (ncm_tx_tasklet [usb_f_ncm]) from [<c0029854>] (tasklet_action+0x94/0xf4)
r5:ee0f8ee0 r4:ee0f8edc
[<c00297c0>] (tasklet_action) from [<c0028f28>] (__do_softirq+0xec/0x220)
r8:c0676098 r7:00000100 r6:c0676088 r5:00000030 r4:eeb86000 r3:40000004
[<c0028e3c>] (__do_softirq) from [<c00292d8>] (irq_exit+0x8c/0xfc)
r10:00000002 r9:60000013 r8:00000001 r7:ee806000 r6:00000000 r5:c0671ac8
r4:00000000
[<c002924c>] (irq_exit) from [<c005b270>] (__handle_domain_irq+0x94/0xb8)
r4:00000000 r3:0000018e
[<c005b1dc>] (__handle_domain_irq) from [<c0009394>] (gic_handle_irq+0x40/0x64)
r8:eea2dd5c r7:eeb87de4 r6:c067c964 r5:eeb87db0 r4:f0002000 r3:eeb87db0
[<c0009354>] (gic_handle_irq) from [<c0012bc0>] (__irq_svc+0x40/0x54)
Exception stack(0xeeb87db0 to 0xeeb87df8)
7da0: eea2dd5c aa05aa04 00000000 00000000
7dc0: eea2dd40 ee0be1c0 ee180e80 eea2dd5c eea2dd5c 60000013 00000002 eeb87e1c
7de0: eeb87e20 eeb87df8 c04751a4 c005586c 60000013 ffffffff
r6:ffffffff r5:60000013 r4:c005586c r3:c04751a4
[<c00557d4>] (do_raw_spin_lock) from [<c04751a4>] (_raw_spin_lock+0x10/0x14)
r9:60000013 r8:ee1d3ab4 r7:eea2dd5c r6:ee180e80 r5:ee0be1c0 r4:eea2dd40
[<c0475194>] (_raw_spin_lock) from [<bf000584>] (tx_complete+0x70/0xd8 [u_ether])
[<bf000514>] (tx_complete [u_ether]) from [<c02d96c8>] (usb_gadget_giveback_request+0x14/0x18)
r7:ee1d3a10 r6:eebaa86c r5:00000000 r4:ee0be1f4
[<c02d96b4>] (usb_gadget_giveback_request) from [<c02d89d8>] (usbhsg_queue_done+0x2c/0x30)
[<c02d89ac>] (usbhsg_queue_done) from [<c02d82bc>] (usbhsf_pkt_handler+0xfc/0x114)
[<c02d81c0>] (usbhsf_pkt_handler) from [<c02d82f4>] (usbhsf_dma_complete+0x20/0x58)
r10:00000000 r9:00000001 r8:00200200 r7:00100100 r6:eebc6cc8 r5:ee9a6a00
r4:eebaa86c
[<c02d82d4>] (usbhsf_dma_complete) from [<c01f2efc>] (usb_dmac_isr_channel_thread+0x8c/0xcc)
r5:eebc6c94 r4:ee28956c
[<c01f2e70>] (usb_dmac_isr_channel_thread) from [<c005c384>] (irq_thread_fn+0x24/0x3c)
r8:c005c360 r7:ee8f6660 r6:ee999640 r5:eeb86000 r4:ee8f6640 r3:c01f2e70
[<c005c360>] (irq_thread_fn) from [<c005c658>] (irq_thread+0xf4/0x16c)
r6:ee8f6640 r5:eeb86000 r4:ee999640 r3:00000004
[<c005c564>] (irq_thread) from [<c003d834>] (kthread+0xf0/0x104)
r10:00000000 r9:00000000 r8:00000000 r7:c005c564 r6:ee8f6640 r5:00000000
r4:ee8f6600
[<c003d744>] (kthread) from [<c000ebe0>] (ret_from_fork+0x14/0x34)
r7:00000000 r6:00000000 r5:c003d744 r4:ee8f6600
=====================
Best regards,
Yoshihiro Shimoda
��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
