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@xxxxxx> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > Cc: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx> > Cc: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Cc: Michal Nazarewicz <mina86@xxxxxxxxxx> > Cc: Robert Baldyga <r.baldyga@xxxxxxxxxxx> > Cc: linux-usb@xxxxxxxxxxxxxxx > Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> Looks good to me. Reviewed-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx> > --- > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
