All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Cole Munz <Munzzyy1@proton.me>
Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3] usb: dwc3: gadget: don't error on dequeue of a completed request
Date: Fri, 11 Sep 2026 23:39:27 +0000	[thread overview]
Message-ID: <aqSLGvMpw62nrToQ@vbox> (raw)
In-Reply-To: <20260905020310.2277320-1-Munzzyy1@proton.me>

On Sat, Sep 05, 2026, Cole Munz wrote:
> On Sat, Sep 05, 2026, Thinh Nguyen wrote:
> > Why is functionfs_unbind() calling usb_ep_dequeue() unconditionally
> > after the request has already completed?
> >
> > The change ce405d561b02 looks like a workaround for a different issue.
> > Maybe the fix belongs in f_fs instead of dwc3?
> 
> The dequeue there does a real job in one case: teardown racing an ep0
> read/write. ffs_ep0_read/write hold ffs->mutex while they sit in
> __ffs_ep0_queue_wait(), so functionfs_unbind() dequeues before taking
> the mutex. The giveback with -ECONNRESET is what wakes the parked
> reader so it can return and drop the mutex, and only then can unbind
> free the request. In every other teardown the request completed long
> ago and the dequeue is a no-op, which is the case dwc3 complains
> about.
> 
> f_fs can tell those apart itself. ep0req is only ever queued from
> __ffs_ep0_queue_wait(), so set a flag there before usb_ep_queue(),
> clear it in ffs_ep0_complete(), and have functionfs_unbind() dequeue
> only when it's set. If the completion fires between the check and the
> dequeue you can still hit the dwc3 error, but that's a rare race
> window where the documented behavior is doing its job, not noise on
> every unbind.
> 
> If that direction looks right to you I'll send it as an f_fs patch,
> Cc'ing Udipto since it revisits ce405d561b02.
> 

I don't feel strongly about the print. Just wanted to raise the question
on the f_fs logic. Downgrading to dev_dbg() is fine. The intention was
to catch gadget driver misuse of dequeue(), not to flag a fatal error.

Thanks,
Thinh

      reply	other threads:[~2026-09-11 23:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 16:13 [PATCH RESEND] usb: dwc3: gadget: don't error on dequeue of a completed request Cole Munz
2026-09-04 16:16 ` Greg Kroah-Hartman
2026-09-04 16:58   ` [PATCH v2] " Cole Munz
2026-09-04 17:07     ` Greg Kroah-Hartman
2026-09-04 20:00       ` [PATCH v3] " Cole Munz
2026-09-04 23:30         ` Thinh Nguyen
2026-09-05  0:02           ` Cole Munz
2026-09-05  1:22             ` Thinh Nguyen
2026-09-05  2:03               ` Cole Munz
2026-09-11 23:39                 ` Thinh Nguyen [this message]

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=aqSLGvMpw62nrToQ@vbox \
    --to=thinh.nguyen@synopsys.com \
    --cc=Munzzyy1@proton.me \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /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.