* [PATCH] functionfs: Avoid aio locking problem
@ 2015-06-22 18:01 John Stultz
2015-07-01 9:42 ` Robert Baldyga
0 siblings, 1 reply; 2+ messages in thread
From: John Stultz @ 2015-06-22 18:01 UTC (permalink / raw
To: lkml
Cc: John Stultz, Felipe Balbi, Al Viro, Andrzej Pietrasiewicz,
Krzysztof Opasiak, Greg Kroah-Hartman, Michal Nazarewicz,
Robert Baldyga, linux-usb
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/140000000000000/0 softirq=7187/7189 fqs=2601
(detected by 3, t=2603 jiffies, g=3028, c=3027, q=474)
Task dump for CPU 0:
adbd R running 0 2791 1 0x00000002
[<c075f234>] (__schedule) from [<ffffffff>] (0xffffffff)
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 <balbi@ti.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Cc: Krzysztof Opasiak <k.opasiak@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Nazarewicz <mina86@mina86.com>
Cc: Robert Baldyga <r.baldyga@samsung.com>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
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)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] functionfs: Avoid aio locking problem
2015-06-22 18:01 [PATCH] functionfs: Avoid aio locking problem John Stultz
@ 2015-07-01 9:42 ` Robert Baldyga
0 siblings, 0 replies; 2+ messages in thread
From: Robert Baldyga @ 2015-07-01 9:42 UTC (permalink / raw
To: John Stultz, lkml
Cc: Felipe Balbi, Al Viro, Andrzej Pietrasiewicz, Krzysztof Opasiak,
Greg Kroah-Hartman, Michal Nazarewicz, linux-usb
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/140000000000000/0 softirq=7187/7189 fqs=2601
> (detected by 3, t=2603 jiffies, g=3028, c=3027, q=474)
> Task dump for CPU 0:
> adbd R running 0 2791 1 0x00000002
> [<c075f234>] (__schedule) from [<ffffffff>] (0xffffffff)
>
> 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 <balbi@ti.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
> Cc: Krzysztof Opasiak <k.opasiak@samsung.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Michal Nazarewicz <mina86@mina86.com>
> Cc: Robert Baldyga <r.baldyga@samsung.com>
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
Looks good to me.
Reviewed-by: Robert Baldyga <r.baldyga@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)
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-07-01 9:42 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-22 18:01 [PATCH] functionfs: Avoid aio locking problem John Stultz
2015-07-01 9:42 ` Robert Baldyga
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.