Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1553059940-127038-1-git-send-email-fei.yang@intel.com> <CALAqxLUW9XCq0WqTzpYGZ7B_4CVdsXY6t_dyuwhnOTJ7HajrwQ@mail.gmail.com>
In-Reply-To: <CALAqxLUW9XCq0WqTzpYGZ7B_4CVdsXY6t_dyuwhnOTJ7HajrwQ@mail.gmail.com>
From:   John Stultz <john.stultz@linaro.org>
Date:   Wed, 20 Mar 2019 11:21:02 -0700
Message-ID: <CALAqxLUwB4vYiCM1Yi2CizMu-FvEoBB-eNqwaTFi2b=gnvCCZQ@mail.gmail.com>
Subject: Re: [PATCH V2] usb: gadget: f_fs: don't free buffer prematurely
To:     fei.yang@intel.com
Cc:     Felipe Balbi <balbi@kernel.org>,
        Greg KH <gregkh@linuxfoundation.org>, andrzej.p@collabora.com,
        Vincent Pelletier <plr.vincent@gmail.com>,
        Linux USB List <linux-usb@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>, Josh Gao <jmgao@google.com>,
        Alistair Strachan <astrachan@google.com>,
        Shen Jing <jingx.shen@intel.com>,
        Alan Stern <stern@rowland.harvard.edu>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 20, 2019 at 9:40 AM John Stultz <john.stultz@linaro.org> wrote:
>
> On Tue, Mar 19, 2019 at 10:32 PM <fei.yang@intel.com> wrote:
>   So while this patch does resolve the issues I was seeing with
> mainline kernels and recent changes to adbd, Josh pointed out that it
> wouldn't resolve the issues I was seeing with older kernels which is
> slightly different (but still related to aio usage).
>
> On the older kernels I'm hitting scheduling while atomic on reboot,
> which seems to be due to ffs_aio_cancel() taking a spinlock then
> calling usb_ep_dequeue() which might sleep.
>
> It seems a fix for this was tried earlier with d52e4d0c0c428 ("usb:
> gadget: ffs: Fix BUG when userland exits with submitted AIO
> transfers") which was then reverted by a9c859033f6e.
>
> Elsewhere it seems the ffs driver takes effort to drop any locks
> before calling usb_ep_dequeue(), so this seems like that should be
> addressed, but it also seems like recent change to the dwc3 driver has
> been made to avoid sleeping in that path (see fec9095bdef4 ("usb:
> dwc3: gadget: remove wait_end_transfer")), which may be why I'm not
> seeing the problem with mainline (and your patch here, of coarse).
> But that also doesn't clarify if its still a potential issue w/
> non-dwc3 platforms.

Felipe: Given Alan's point, does it make sense to mark the commits
that remove the possible sleep from wait_event_lock_irq() in
dwc3_gadget_ep_dequeue()  for -stable?

Against 4.19.30, the following set manages to cherry-pick cleanly:
git cherry-pick 1a22ec643580626f439c8583edafdcc73798f2fb
git cherry-pick 09fe1f8d7e2f461275b1cdd832f2cfa5e9be346d
git cherry-pick c3acd59014148470dc58519870fbc779785b4bf7
git cherry-pick 7746a8dfb3f9c91b3a0b63a1d5c2664410e6498d
git cherry-pick d5443bbf5fc8f8389cce146b1fc2987cdd229d12
git cherry-pick d4f1afe5e896c18ae01099a85dab5e1a198bd2a8
git cherry-pick fec9095bdef4e7c988adb603d0d4f92ee735d4a1
# To get things building, revert modified -stable fix
git revert 25ad17d
#pick actual upstream fix replacing the previous
git cherry-pick bd6742249b9ca918565e4e3abaa06665e587f4b5

(Though I'm always a bit hesitant with -stable backports on subsystems
I don't know well. So I'm not sure if this set is fully correct.)

This set seems to avoid the crash on reboot I was seeing.

And of course, I'm sure getting that set backported to 4.14 and 4.9
(and maybe even 4.4, I need to check) will be less clean.

thanks
-john