All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: fei.yang@intel.com, 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>
Subject: Re: [PATCH V2] usb: gadget: f_fs: don't free buffer prematurely
Date: Wed, 20 Mar 2019 09:55:22 -0700	[thread overview]
Message-ID: <CALAqxLUdNe5PUnqM8MBdQaGkMU0QGuDjpWL0VeSCH_fx2jWGWg@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1903201247570.1538-100000@iolanthe.rowland.org>

On Wed, Mar 20, 2019 at 9:52 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, 20 Mar 2019, John Stultz wrote:
>
> > Hey Fei,
> >   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.
> >
> > So for older kernels, do you have a suggestion of which approach is
> > advised?  Does usb_ep_dequeue need to avoid sleeping or do we need to
> > rework the ffs_aio_cancel logic?
>
> usb_ep_dequeue can be called in interrupt context, meaning it is never
> allowed to sleep.  This is mentioned in the kerneldoc:

Thanks for the clarification! Sorry I didn't see the kernel doc!
-john

WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: fei.yang@intel.com, 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>
Subject: [V2] usb: gadget: f_fs: don't free buffer prematurely
Date: Wed, 20 Mar 2019 09:55:22 -0700	[thread overview]
Message-ID: <CALAqxLUdNe5PUnqM8MBdQaGkMU0QGuDjpWL0VeSCH_fx2jWGWg@mail.gmail.com> (raw)

On Wed, Mar 20, 2019 at 9:52 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, 20 Mar 2019, John Stultz wrote:
>
> > Hey Fei,
> >   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.
> >
> > So for older kernels, do you have a suggestion of which approach is
> > advised?  Does usb_ep_dequeue need to avoid sleeping or do we need to
> > rework the ffs_aio_cancel logic?
>
> usb_ep_dequeue can be called in interrupt context, meaning it is never
> allowed to sleep.  This is mentioned in the kerneldoc:

Thanks for the clarification! Sorry I didn't see the kernel doc!
-john

  reply	other threads:[~2019-03-20 16:55 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20  5:32 [PATCH V2] usb: gadget: f_fs: don't free buffer prematurely fei.yang
2019-03-20  5:32 ` [V2] " fei.yang
2019-03-20  5:56 ` [PATCH V2] " Josh Gao
2019-03-20  5:56   ` [V2] " Josh Gao
2019-03-20  5:59   ` [PATCH V2] " Josh Gao
2019-03-20  5:59     ` [V2] " Josh Gao
2019-03-20 17:30     ` [PATCH V2] " Yang, Fei
2019-03-20 17:30       ` [V2] " fei.yang
2019-03-20 16:40 ` [PATCH V2] " John Stultz
2019-03-20 16:40   ` [V2] " John Stultz
2019-03-20 16:52   ` [PATCH V2] " Alan Stern
2019-03-20 16:52     ` [V2] " Alan Stern
2019-03-20 16:55     ` John Stultz [this message]
2019-03-20 16:55       ` John Stultz
2019-03-20 18:21   ` [PATCH V2] " John Stultz
2019-03-20 18:21     ` [V2] " John Stultz
2019-04-02  3:26     ` [PATCH V2] " John Stultz
2019-04-02  3:26       ` [V2] " John Stultz
2019-04-24 16:50       ` [PATCH V2] " Greg KH
2019-04-24 16:50         ` [V2] " Greg Kroah-Hartman
2019-04-25  3:53         ` [PATCH V2] " John Stultz
2019-04-25  3:53           ` [V2] " John Stultz
2019-04-25 16:01           ` [PATCH V2] " Yang, Fei
2019-04-25 16:01             ` [V2] " fei.yang
2019-04-25 16:19             ` [PATCH V2] " John Stultz
2019-04-25 16:19               ` [V2] " John Stultz
2019-03-20 23:28   ` [PATCH V2] " Yang, Fei
2019-03-20 23:28     ` [V2] " fei.yang
2019-03-20 23:42     ` [PATCH V2] " John Stultz
2019-03-20 23:42       ` [V2] " John Stultz
2019-04-02  3:25 ` [PATCH V2] " John Stultz
2019-04-02  3:25   ` [V2] " John Stultz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALAqxLUdNe5PUnqM8MBdQaGkMU0QGuDjpWL0VeSCH_fx2jWGWg@mail.gmail.com \
    --to=john.stultz@linaro.org \
    --cc=andrzej.p@collabora.com \
    --cc=astrachan@google.com \
    --cc=balbi@kernel.org \
    --cc=fei.yang@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jingx.shen@intel.com \
    --cc=jmgao@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=plr.vincent@gmail.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.