Hi John,
On 06/22/2015 08:01 PM, John Stultz wrote:
The functionfs aio logic seems broken. When using functionfs,
I was seeing frequent hangs, and enabling spinlock debugging,
I got:
g_ffs gadget: g_ffs ready
ci_hdrc ci_hdrc.0: CI_HDRC_CONTROLLER_RESET_EVENT received
BUG: spinlock lockup suspected on CPU#0, adbd/2791
lock: 0xe7764880, .magic: e7764880, .owner: none/-1, .owner_cpu: -407539900
CPU: 0 PID: 2791 Comm: adbd Not tainted 4.1.0-rc1-00032-g359b12f #147
Hardware name: Qualcomm (Flattened Device Tree)
[c0216ac8] (unwind_backtrace) from [c02136a8] (show_stack+0x10/0x14)
[c02136a8] (show_stack) from [c075d9fc] (dump_stack+0x70/0xbc)
[c075d9fc] (dump_stack) from [c026ef90] (do_raw_spin_lock+0x114/0x1a0)
[c026ef90] (do_raw_spin_lock) from [c0764cb8]
(_raw_spin_lock_irqsave+0x50/0x5c)
[c0764cb8] (_raw_spin_lock_irqsave) from [c037c1a0]
(kiocb_set_cancel_fn+0x1c/0x60)
[c037c1a0] (kiocb_set_cancel_fn) from [c05ae568]
(ffs_epfile_read_iter+0x8c/0x140)
[c05ae568] (ffs_epfile_read_iter) from [c0332018] (__vfs_read+0xb0/0xd4)
[c0332018] (__vfs_read) from [c0332ef8] (vfs_read+0x7c/0x100)
[c0332ef8] (vfs_read) from [c0332fbc] (SyS_read+0x40/0x8c)
[c0332fbc] (SyS_read) from [c020ff20] (ret_fast_syscall+0x0/0x4c)
INFO: rcu_preempt detected stalls on CPUs/tasks:
0: (1 GPs behind) idle=805/140/0 softirq=7187/7189 fqs=2601
(detected by 3, t=2603 jiffies, g=3028, c=3027, q=474)
Task dump for CPU 0:
adbdR running 0 2791 1 0x0002
[c075f234] (__schedule) from [] (0x)
Looking at the code, the __vfs_read() calls new_sync_read(),
which allocates a struct kiocb kiocb on the stack and passes
it to the ffs_epfile_read_iter() funciton. That then calls
kiocb_set_cancel_fn() passing a pointer to that kiocb. However,
kiocb_set_cancel_fn() assumes the kiocb is a sub-element of a
struct aio_kiocb, and it tries to grab the kioctx from that
parent structure. However it seems there is no aio_kiocb
structure here, so the spin_lock_irqsave hangs trying to lock
random data on the stack.
This patch avoids the issue, by only calling kiocb_set_cancel_fn
if the aio flag is set.
Cc: Felipe Balbi ba...@ti.com
Cc: Al Viro v...@zeniv.linux.org.uk
Cc: Andrzej Pietrasiewicz andrze...@samsung.com
Cc: Krzysztof Opasiak k.opas...@samsung.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Cc: Michal Nazarewicz min...@mina86.com
Cc: Robert Baldyga r.bald...@samsung.com
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz john.stu...@linaro.org
Looks good to me.
Reviewed-by: Robert Baldyga r.bald...@samsung.com
---
drivers/usb/gadget/function/f_fs.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 3507f88..d2434c9 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -924,7 +924,8 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb,
struct iov_iter *from)
kiocb-private = p;
- kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
+ if (p-aio)
+ kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
res = ffs_epfile_io(kiocb-ki_filp, p);
if (res == -EIOCBQUEUED)
@@ -968,7 +969,8 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb,
struct iov_iter *to)
kiocb-private = p;
- kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
+ if (p-aio)
+ kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
res = ffs_epfile_io(kiocb-ki_filp, p);
if (res == -EIOCBQUEUED)
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
