From patchwork Sat Jun 13 00:51:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: John Stultz X-Patchwork-Id: 49837 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-lb0-f198.google.com (mail-lb0-f198.google.com [209.85.217.198]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 86F5720C81 for ; Sat, 13 Jun 2015 00:51:21 +0000 (UTC) Received: by lbbqq2 with SMTP id qq2sf13546895lbb.0 for ; Fri, 12 Jun 2015 17:51:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:content-type:sender:precedence :list-id:x-original-sender:x-original-authentication-results :mailing-list:list-post:list-help:list-archive:list-unsubscribe; bh=CnVkQ/4oMvcMj5zJXd5P4KbYwLH3iG2LwtHRnYaAStg=; b=gstTSIWnd7L/VWBCyKEAdGgB2fkCBS/qApJhZUPDAiB0zNP3PqhjgwX/74v3zJ6TFT Yw4+Rap24v1JQQqEwAOvaWg4MXBAe8zJ/Wo4s1XtpnyV/Zv5xoh4syFp3GvnQ6UIPnHU s0rX7bEvXi05rEXme1ImpifIG1aJO9iPVf+S1hlAy2C0rnDP7+9X19N9JvRzN+RwlfaM 27WzF4seThnk2YvSSn7OmDBFoV41wxAFvax2HUvWyrBwjNJG5rfUp4hRxfyqr3TeWz1K N3qx4pSw4NVSYiPVZXJ/BlPYm0tfgj17jJ1k/EiGQEFeLOo0qM0nevYD7ikAJiumxqWb T3Qg== X-Gm-Message-State: ALoCoQk+LjeHeXKJLESWWazHD7ZbK1GMnKNPPInMkMkgQXhtsUfYhZP34K1S4dmXmWWaTEAU5OTB X-Received: by 10.180.11.101 with SMTP id p5mr6822494wib.3.1434156680258; Fri, 12 Jun 2015 17:51:20 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.3.133 with SMTP id c5ls544122lac.68.gmail; Fri, 12 Jun 2015 17:51:20 -0700 (PDT) X-Received: by 10.152.87.164 with SMTP id az4mr465330lab.76.1434156680095; Fri, 12 Jun 2015 17:51:20 -0700 (PDT) Received: from mail-la0-f54.google.com (mail-la0-f54.google.com. [209.85.215.54]) by mx.google.com with ESMTPS id ug3si4672973lbb.130.2015.06.12.17.51.19 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 12 Jun 2015 17:51:19 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.54 as permitted sender) client-ip=209.85.215.54; Received: by lagv1 with SMTP id v1so322937lag.1 for ; Fri, 12 Jun 2015 17:51:19 -0700 (PDT) X-Received: by 10.152.87.164 with SMTP id az4mr465314lab.76.1434156679738; Fri, 12 Jun 2015 17:51:19 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp898526lbb; Fri, 12 Jun 2015 17:51:18 -0700 (PDT) X-Received: by 10.70.91.99 with SMTP id cd3mr8602202pdb.166.1434156677783; Fri, 12 Jun 2015 17:51:17 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ig4si7320792pbb.82.2015.06.12.17.51.16; Fri, 12 Jun 2015 17:51:17 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-usb-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753689AbbFMAvP (ORCPT + 4 others); Fri, 12 Jun 2015 20:51:15 -0400 Received: from mail-vn0-f41.google.com ([209.85.216.41]:37090 "EHLO mail-vn0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752097AbbFMAvN (ORCPT ); Fri, 12 Jun 2015 20:51:13 -0400 Received: by vnbg62 with SMTP id g62so8488541vnb.4 for ; Fri, 12 Jun 2015 17:51:13 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.52.114.230 with SMTP id jj6mr30535168vdb.66.1434156672911; Fri, 12 Jun 2015 17:51:12 -0700 (PDT) Received: by 10.52.36.203 with HTTP; Fri, 12 Jun 2015 17:51:12 -0700 (PDT) In-Reply-To: References: Date: Fri, 12 Jun 2015 17:51:12 -0700 Message-ID: Subject: Re: Locking issues w/ functionfs gadget and aio? From: John Stultz To: Felipe Balbi , Al Viro , Andrzej Pietrasiewicz , Krzysztof Opasiak Cc: Greg Kroah-Hartman , Michal Nazarewicz , Robert Baldyga , lkml , linux-usb@vger.kernel.org Sender: linux-usb-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: john.stultz@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.54 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Mon, Jun 8, 2015 at 6:14 PM, John Stultz wrote: > After setting up functionfs for adb w/ 4.1-rc7, I noticed some flakey behavior. > I enabled some lock debugging and got the following: > > [ 91.648093] read strings > [ 91.650264] g_ffs gadget: g_ffs ready > [ 91.652551] ci_hdrc ci_hdrc.0: CI_HDRC_CONTROLLER_RESET_EVENT received > [ 96.068693] BUG: spinlock lockup suspected on CPU#0, adbd/2791 > [ 96.068751] lock: 0xe7764880, .magic: e7764880, .owner: /-1, > .owner_cpu: -407539900 > [ 96.073448] CPU: 0 PID: 2791 Comm: adbd Not tainted > 4.1.0-rc1-00032-g359b12f #147 > [ 96.081688] Hardware name: Qualcomm (Flattened Device Tree) > [ 96.089266] [] (unwind_backtrace) from [] > (show_stack+0x10/0x14) > [ 96.094635] [] (show_stack) from [] > (dump_stack+0x70/0xbc) > [ 96.102627] [] (dump_stack) from [] > (do_raw_spin_lock+0x114/0x1a0) > [ 96.109661] [] (do_raw_spin_lock) from [] > (_raw_spin_lock_irqsave+0x50/0x5c) > [ 96.117560] [] (_raw_spin_lock_irqsave) from [] > (kiocb_set_cancel_fn+0x1c/0x60) > [ 96.126519] [] (kiocb_set_cancel_fn) from [] > (ffs_epfile_read_iter+0x8c/0x140) > [ 96.135289] [] (ffs_epfile_read_iter) from [] > (__vfs_read+0xb0/0xd4) > [ 96.144290] [] (__vfs_read) from [] (vfs_read+0x7c/0x100) > [ 96.152535] [] (vfs_read) from [] (SyS_read+0x40/0x8c) > [ 96.159571] [] (SyS_read) from [] > (ret_fast_syscall+0x0/0x4c) > [ 117.678633] INFO: rcu_preempt detected stalls on CPUs/tasks: > [ 117.683069] 0: (1 GPs behind) idle=805/140000000000000/0 > softirq=7187/7189 fqs=2601 > [ 117.683316] (detected by 3, t=2603 jiffies, g=3028, c=3027, q=474) > [ 117.692345] Task dump for CPU 0: > [ 117.697202] adbd R running 0 2791 1 0x00000002 > [ 117.704296] [] (__schedule) from [] (0xffffffff) > > > It seems we're stuck on the kioctx.ctx_lock, and reviewing that lock > usage I don't see any problems in fs/aio.c right off. > > So I'm guessing the f_fs.c code is somehow not initializing the lock > structure, or maybe calling kiocb_set_cancel_fn() from the wrong > context? So looking further at this, it seems that 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. I'm not super sure what the right fix is, but if do something like the following (sorry, whitespace corrupted via copy/paste), I don't seem to run into the problem. 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) Is there a better solution here? I'm not sure I see if the is_sync_kiocb(kiocb) check would ever be false from here, so I'm not sure if all the p->aio checking is really needed or not. This issue seems to have been introduced with 70e60d917e91fff (gadget/function/f_fs.c: switch to ->{read,write}_iter()) thanks -john --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 3507f88..e322818 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);