[PATCH 1/2] Revert "usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers"

From: Shen Jing This reverts commit b4194da3f9087dd38d91b40f9bec42d59ce589a8 since it causes list corruption followed by kernel panic: Workqueue: adb ffs_aio_cancel_worker RIP: 0010:__list_add_valid+0x4d/0x70 Call Trace: insert_work+0x47/0xb0 __queue_work+0xf6/0x400 queue_work_on+0x65/0x70

[PATCH] Revert "usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers"

From: Shen Jing This reverts commit b4194da3f9087dd38d91b40f9bec42d59ce589a8 since it causes list corruption followed by kernel panic: Workqueue: adb ffs_aio_cancel_worker RIP: 0010:__list_add_valid+0x4d/0x70 Call Trace: insert_work+0x47/0xb0 __queue_work+0xf6/0x400 queue_work_on+0x65/0x70

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On 06/29/2018 08:32 AM, Felipe Balbi wrote: > > Hi, > > Alan Stern writes: >> On Thu, 21 Jun 2018, Roger Quadros wrote: >> > Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() > directly from here? >> What is the purpose of the spin_lock()? I agree

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

hash as of Linus' master): > commit d52e4d0c0c428bf2ba35074a7495cdb28e2efbae > Author: Vincent Pelletier > Date: Wed Jun 13 11:05:06 2018 + > > usb: gadget: ffs: Fix BUG when userland exits with submitted AIO > transfers > > This bug happens only when t

RE: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hi, Vincent: We tried my patch doesn't fix the issue totally, we still see the kernel panic. currently solution in our test is: Rvert "usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers" and only add the below patch to use the in

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

h. It is only *if* dwc3 cancel stops scheduling that I believe my patch should be reverted (here is the hash as of Linus' master): commit d52e4d0c0c428bf2ba35074a7495cdb28e2efbae Author: Vincent Pelletier Date: Wed Jun 13 11:05:06 2018 + usb: gadget: ffs: Fix BUG when userland

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On Thu, 2 Aug 2018 00:45:14 +, "He, Bo" wrote: > Your patch fix the issue BUG: scheduling while atomic: Yes, although from my understanding of Felipe's answer, the actual bug is the "scheduling" part (sleeping in dwc3 UDC) rather than the "atomic" part. So my patch addresses, still if my

RE: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

From: Vincent Pelletier Sent: Wednesday, August 1, 2018 11:04 PM To: Felipe Balbi Cc: Alan Stern ; Roger Quadros ; Lars-Peter Clausen ; Sam Protsenko ; linux-usb@vger.kernel.org; Praneeth Bajjuri ; He, Bo ; Bai, Jie A Subject: Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO tra

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hello, Bo He, CC'ed, found a regression introduced in my patch, discussed in this thread, and submitted a patch: Subject: [PATCH] fix panic at pwq_activate_delayed_work. Date: Wed, 1 Aug 2018 10:14:38 + Message-ID: On Fri, 29 Jun 2018 09:32:43 +0300, Felipe Balbi wrote: > Hmm,

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hi, Alan Stern writes: > On Thu, 21 Jun 2018, Roger Quadros wrote: > >> >> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() >> >> directly from here? >> >>> What is the purpose of the spin_lock()? >> > >> > I agree that the lock doesn't seem to be necessary. But I

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On Thu, 21 Jun 2018, Roger Quadros wrote: > >> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() > >> directly from here? > >>> What is the purpose of the spin_lock()? > > > > I agree that the lock doesn't seem to be necessary. But I believe the whole > > thing is

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On 21/06/18 13:52, Lars-Peter Clausen wrote: > On 06/21/2018 10:29 AM, Roger Quadros wrote: > [...] static int ffs_aio_cancel(struct kiocb *kiocb) { struct ffs_io_data *io_data = kiocb->private; - struct ffs_epfile *epfile = kiocb->ki_filp->private_data; +

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On 06/21/2018 10:29 AM, Roger Quadros wrote: [...] >>> static int ffs_aio_cancel(struct kiocb *kiocb) >>> { >>> struct ffs_io_data *io_data = kiocb->private; >>> - struct ffs_epfile *epfile = kiocb->ki_filp->private_data; >>> + struct ffs_data *ffs = io_data->ffs; >>>

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

+Lars-Peter Vincent, On 14/06/18 16:23, Sam Protsenko wrote: > + Roger Quadros > + Praneeth Bajjuri > > Tested-by: Sam Protsenko > > I've tested it on X15 board (DWC3 controller) on Android master, by > doing "adb root". Without this patch I see backtrace and kernel panic > (the same error as

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hi Felipe, If there is no objections, can we please pull this patch? It fixes kernel panic for us (scheduling in atomic context). Thanks! On 14 June 2018 at 16:23, Sam Protsenko wrote: > + Roger Quadros > + Praneeth Bajjuri > > Tested-by: Sam Protsenko > > I've tested it on X15 board (DWC3

Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

+ Roger Quadros + Praneeth Bajjuri Tested-by: Sam Protsenko I've tested it on X15 board (DWC3 controller) on Android master, by doing "adb root". Without this patch I see backtrace and kernel panic (the same error as described in commit message). When this patch is applied, everything works

Re: Regarding usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hello, On Sat, 2 Jun 2018 01:02:24 +0300, Sam Protsenko wrote: > Hi Vincent, > > The issue you mentioned in your RFC patch is also reproducing on TI > X15 board (dwc3) with Android master, when doing "adb root" command. > Your patch fixes it just fine, for which I really thankful. Thanks a lot

usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

This bug happens only when the UDC needs to sleep during usb_ep_dequeue, as is the case for (at least) dwc3. [ 382.200896] BUG: scheduling while atomic: screen/1808/0x0100 [ 382.207124] 4 locks held by screen/1808: [ 382.211266] #0: (rcu_callback){}, at: []

Regarding usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hi Vincent, The issue you mentioned in your RFC patch is also reproducing on TI X15 board (dwc3) with Android master, when doing "adb root" command. Your patch fixes it just fine, for which I really thankful. Do you know is there anything preventing it to be merged? I have X15 (dwc3) and

Re: [RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hello, On Sun, 7 Jan 2018 12:48:35 +, Vincent Pelletier wrote: > Some update: this patch looks good on DWC3: both unplugging and > unbinding the UDC while AOI transfers are queued works fine. > > What I intended to do is to also test it on DWC2, so I bought a >

Re: [RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On Fri, 1 Dec 2017 00:14:52 +, Vincent Pelletier wrote: > I'll play some more with a kernel with this patch > to see if it breaks, and will resubmit if it looks fine. Some update: this patch looks good on DWC3: both unplugging and unbinding the UDC while AOI transfers

Re: [RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

Hello, On Thu, 30 Nov 2017 13:59:30 -0500 (EST), Alan Stern wrote: > This is a little misleading. The req->complete callback will be called > whether the request is dequeued or not. The real race is between > usb_ep_dequeue and completion of the transfer. Actually,

Re: [RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

On Thu, 30 Nov 2017, Vincent Pelletier wrote: > (tagging subject more ostensibly as RFC) > > On Tue, 28 Nov 2017 16:18:36 +, Vincent Pelletier > wrote: > > - This change should have no effect on existing cancel/complete race > > condition, which I expect is already

[RFC] Re: usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

(tagging subject more ostensibly as RFC) On Tue, 28 Nov 2017 16:18:36 +, Vincent Pelletier wrote: > - This change should have no effect on existing cancel/complete race > condition, which I expect is already handled in AIO. If not, then it's > yet another

usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

!! This is an RFC patch - hence sign-off absence !! This bug happens only when the UDC needs to sleep during usb_ep_dequeue, as is the case for (at least) dwc3. RFC: - I do not understand what eps_lock is supposed to protect in original ffs_aio_cancel implementation, especially once one