From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S933526AbcI0KBl (ORCPT <rfc822;w@1wt.eu>);
        Tue, 27 Sep 2016 06:01:41 -0400
Received: from mga14.intel.com ([192.55.52.115]:28298 "EHLO mga14.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753112AbcI0KBb (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Tue, 27 Sep 2016 06:01:31 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,404,1470726000"; 
   d="asc'?scan'208";a="1062840290"
From: Felipe Balbi <felipe.balbi@linux.intel.com>
To: Chen Yu <chenyu56@huawei.com>, gregkh@linuxfoundation.org
Cc: wangbinghui@hisilicon.com, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org, John Stultz <john.stultz@linaro.org>,
        Amit Pundir <amit.pundir@linaro.org>,
        Guodong Xu <guodong.xu@linaro.org>, mina86@mina86.com
Subject: Re: BUG: scheduling while atomic in f_fs when gadget remove driver
In-Reply-To: <205cfce1-d54c-262d-f939-ad9f37b0c52c@huawei.com>
References: <205cfce1-d54c-262d-f939-ad9f37b0c52c@huawei.com>
User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.3 (x86_64-pc-linux-gnu)
Date: Tue, 27 Sep 2016 13:01:10 +0300
Message-ID: <878tud4q6x.fsf@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
        micalg=pgp-sha256; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable


Hi,

Chen Yu <chenyu56@huawei.com> writes:
> Hi All,
>
> I'm working on Hikey board based around the HiSilicon Kirin 620, with
> linaro kernel version 4.8.rc1 and I get below BUG error while
> extracting USB cable from PC.

which peripheral controller does this one have? Is it dwc3?

I'm very interested in knowing about throughtput of adb push with dwc3 + f_=
fs.

Also, do you know if adb can run outside of android environment? I've
been looking for a proper functionfs user for quite some time now :-(

> The funtion using f_fs is adb and usb_gadget_unregister_driver will be
> called after extracting USB cable from PC.
>
> [   89.456512s][pid:1,cpu1,init]BUG: scheduling while atomic: init/1/0x00=
000002
> [   89.456573s]Modules linked in:
> [   89.456604s]Preemption disabled at:[<ffffffc0006a6dc0>] composite_disc=
onnect+0x30/0xac
> [   89.456665s][pid:1,cpu1,init]TGID: 1 Comm: init
> [   89.456695s][pid:1,cpu1,init]Call trace:
> [   89.456726s][pid:1,cpu1,init][<ffffffc00008a5e0>] dump_backtrace+0x0/0=
x15c
> [   89.456756s][pid:1,cpu1,init][<ffffffc00008a75c>] show_stack+0x20/0x28
> [   89.456756s][pid:1,cpu1,init][<ffffffc001153714>] dump_stack+0x84/0xa8
> [   89.456787s][pid:1,cpu1,init][<ffffffc0000cfc5c>] __schedule_bug+0x88/=
0xdc
> [   89.456817s][pid:1,cpu1,init][<ffffffc00115c4f0>] __schedule+0x714/0x8=
54
> [   89.456817s][pid:1,cpu1,init][<ffffffc00115c678>] schedule+0x48/0xa4
> [   89.456817s][pid:1,cpu1,init][<ffffffc00115cbf0>] schedule_preempt_dis=
abled+0x4c/0xf4
> [   89.456848s][pid:1,cpu1,init][<ffffffc00115ea90>] __mutex_lock_slowpat=
h+0xbc/0x1a4
> [   89.456878s][pid:1,cpu1,init][<ffffffc00115ebd8>] mutex_lock+0x60/0x64
> [   89.456878s][pid:1,cpu1,init][<ffffffc0006beb00>] ffs_func_eps_disable=
.isra.17+0x54/0x114
> [   89.456909s][pid:1,cpu1,init][<ffffffc0006c05a4>] ffs_func_disable+0x3=
0/0xa0
> [   89.456909s][pid:1,cpu1,init][<ffffffc0006a6c4c>] reset_config.isra.8+=
0x44/0x78
> [   89.456939s][pid:1,cpu1,init][<ffffffc0006a6dd8>] composite_disconnect=
+0x48/0xac
> [   89.456939s][pid:1,cpu1,init][<ffffffc0006aafd4>] android_disconnect+0=
x48/0x54
> [   89.456970s][pid:1,cpu1,init][<ffffffc0006ad9d0>] usb_gadget_remove_dr=
iver+0x58/0xa0
> [   89.456970s][pid:1,cpu1,init][<ffffffc0006ada90>] usb_gadget_unregiste=
r_driver+0x78/0xc4
>
> I checked the codes of composite_disconnect and found
> spin_lock_irqsave called before reset_config in which
> ffs_func_eps_disable is called.
>
> void composite_disconnect(struct usb_gadget *gadget)
> {
>         struct usb_composite_dev        *cdev =3D get_gadget_data(gadget);
>         unsigned long                   flags;
>
>         /* REVISIT:  should we have config and device level
>          * disconnect callbacks?
>          */
>         spin_lock_irqsave(&cdev->lock, flags);
>         if (cdev->config)
>                 reset_config(cdev);
>         if (cdev->driver->disconnect)
>                 cdev->driver->disconnect(cdev);
>         spin_unlock_irqrestore(&cdev->lock, flags);
> }
>
> static void ffs_func_eps_disable(struct ffs_function *func)
> {
>         struct ffs_ep *ep         =3D func->eps;
>         struct ffs_epfile *epfile =3D func->ffs->epfiles;
>         unsigned count            =3D func->ffs->eps_count;
>         unsigned long flags;
>
>         do {
>                 if (epfile)
>                         mutex_lock(&epfile->mutex);
>                 spin_lock_irqsave(&func->ffs->eps_lock, flags);
>                 /* pending requests get nuked */
>                 if (likely(ep->ep))
>                         usb_ep_disable(ep->ep);
>                 ++ep;
>                 spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
>
>                 if (epfile) {
>                         epfile->ep =3D NULL;
>                         kfree(epfile->read_buffer);
>                         epfile->read_buffer =3D NULL;
>                         mutex_unlock(&epfile->mutex);
>                         ++epfile;
>                 }
>         } while (--count);
> }
>
> Should the epfile->read_buffer be cleared another place and the
> mutex_lock can be removed in ffs_func_eps_disable?

You are correct. There's a bug there. Can you try to propose a fix for
it?

thanks

=2D-=20
balbi

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIcBAEBCAAGBQJX6kNmAAoJEMy+uJnhGpkGVfsP/1OZbcMxPvzzIZDF411/P+3w
syZiX+mQC8bsfpkht47z4xag76y1uLcuoz+6Qfel7OcHeuYiCwv/2G4a1zvF+ugs
Hvk7EtziQuRbZypNZbzJUm1/Serlj7DLMlReBo/vqAJPtAfrGNmt3QxjOfZeNGl1
r7Pfm/pUIy0FFfTEINsLo8IAKbowBjGX7dq3QlB1OTZOIORp1MRc5N/AQbLIs3jo
df0XjxcQsm0z6UnkLS/vpyy+cSde25xjVvYY//09xAIFImzi9fLNvMLateIXKuWx
MjN+lyhzRD6akPhdbTwFmxAA7rl0YIHnr4blbJXF+d0Flh5dnIHmilX+F3ttIi9F
sL8msAZZOu8UByggy0UHnR6a/Wla1IFbmzn3+mbcAfy9LjW0RYAaco0yrYfzS5pz
TsBtyAYavHCWlkY6tKu2/NqCH0j57SX/jg8dOnF5ATXoCE03WMhexMsoSt32b9X3
EoaOeBCEblXWXoVOa2MIekTmMzzS9xqksJjKQpBON7EqokUu/Q3DTfHEvSb+afzF
D6T0up5aO9CtDqlPKplyWa/rf6zsBjxw9+DQJRPrnNn649rLpwykrheougJsoq6b
hL3eO2Mykyvvx+U6eiazG/xW1oB5oPPbAcvIOPsYnmcMJbbDiPUAsFrmSHJW06fd
ZZENS3LKlNSdVT20TIbN
=TET7
-----END PGP SIGNATURE-----
--=-=-=--