From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753615AbbGAJmY (ORCPT ); Wed, 1 Jul 2015 05:42:24 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:31188 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751842AbbGAJmO (ORCPT ); Wed, 1 Jul 2015 05:42:14 -0400 X-AuditID: cbfec7f4-f79c56d0000012ee-d7-5593b5f4fcbc Message-id: <5593B5EE.5010400@samsung.com> Date: Wed, 01 Jul 2015 11:42:06 +0200 From: Robert Baldyga User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-version: 1.0 To: John Stultz , lkml Cc: Felipe Balbi , Al Viro , Andrzej Pietrasiewicz , Krzysztof Opasiak , Greg Kroah-Hartman , Michal Nazarewicz , linux-usb@vger.kernel.org Subject: Re: [PATCH] functionfs: Avoid aio locking problem References: <1434996064-20284-1-git-send-email-john.stultz@linaro.org> In-reply-to: <1434996064-20284-1-git-send-email-john.stultz@linaro.org> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrNLMWRmVeSWpSXmKPExsVy+t/xa7pftk4ONfh+mtdi1st2FouD9+st mhevZ7M481vX4vbEaWwWl3fNYbNYtKyV2WLB8RZWi/N/j7M6cHrcubaHzWP/3DXsHuv+vGLy 6NuyitHj+I3tTB6fN8l5bHrylimAPYrLJiU1J7MstUjfLoEro2PpNdaCS7IVj7++ZGpg/CzR xcjJISFgInH6+kdmCFtM4sK99WxdjFwcQgJLGSUmn7jOCOE8Y5TYt2QBK0gVr4CWxLMnZ9hA bBYBVYkrt9YwgthsAjoSW75PALI5OEQFIiS6T1RClAtK/Jh8jwUkLCLgK7H5pxPISGaB6UwS rfNa2UFqhAWsJCZf/8QEYgsJuEns3LeaBcTmFHCXeL/4AVgvs4CexP2LWiBhZgF5ic1r3jJP YBSYhWTDLISqWUiqFjAyr2IUTS1NLihOSs811CtOzC0uzUvXS87P3cQIiYIvOxgXH7M6xCjA wajEwysgNjlUiDWxrLgy9xCjBAezkgjvl6lAId6UxMqq1KL8+KLSnNTiQ4zSHCxK4rxzd70P ERJITyxJzU5NLUgtgskycXBKNTAGfFJxT+0LuL1u9UKXs4+WMwqwdxzyWxXQ7Fe+K3lJ65Sc DRZndlwWfzhn//a2DZrFv+Xe65mUhGs992bNq/jTeab2V4XbVCZvsc6WWPfbx3fayKZuuHvQ Z8OU88tVVn6ebJjUIC6/z2Pl5ci8xyZRq4veth9r+7BBtW5mfa7u25iFGauTpqgqsRRnJBpq MRcVJwIAdVmDh34CAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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: /-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) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x70/0xbc) > [] (dump_stack) from [] (do_raw_spin_lock+0x114/0x1a0) > [] (do_raw_spin_lock) from [] (_raw_spin_lock_irqsave+0x50/0x5c) > [] (_raw_spin_lock_irqsave) from [] (kiocb_set_cancel_fn+0x1c/0x60) > [] (kiocb_set_cancel_fn) from [] (ffs_epfile_read_iter+0x8c/0x140) > [] (ffs_epfile_read_iter) from [] (__vfs_read+0xb0/0xd4) > [] (__vfs_read) from [] (vfs_read+0x7c/0x100) > [] (vfs_read) from [] (SyS_read+0x40/0x8c) > [] (SyS_read) from [] (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 > [] (__schedule) from [] (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 > Cc: Al Viro > Cc: Andrzej Pietrasiewicz > Cc: Krzysztof Opasiak > Cc: Greg Kroah-Hartman > Cc: Michal Nazarewicz > Cc: Robert Baldyga > Cc: linux-usb@vger.kernel.org > Signed-off-by: John Stultz Looks good to me. Reviewed-by: Robert Baldyga > --- > 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) >