From: Greg KH <gregkh@linuxfoundation.org>
To: "Bjørn Mork" <bjorn@mork.no>
Cc: Oliver Neukum <oneukum@suse.com>,
Aleksander Morgado <aleksandermj@chromium.org>,
linux-usb@vger.kernel.org, linux@roeck-us.net,
linux-kernel@vger.kernel.org, ejcaruso@chromium.org
Subject: Re: [PATCH] usb: cdc-wdm: close race between read and workqueue
Date: Mon, 15 Apr 2024 11:52:49 +0200 [thread overview]
Message-ID: <2024041536-simile-flatly-824a@gregkh> (raw)
In-Reply-To: <878r1fht93.fsf@miraculix.mork.no>
On Mon, Apr 15, 2024 at 11:26:00AM +0200, Bjørn Mork wrote:
> Oliver Neukum <oneukum@suse.com> writes:
> > On 15.04.24 08:47, Bjørn Mork wrote:
> >
> >> urb from service_outstanding_interrupt(). That's why it was added. See
> >> the explanation Robert wrote when introducing it:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/class/cdc-wdm.c?id=c1da59dad0ebd3f9bd238f3fff82b1f7ffda7829
> >
> > Well, the explanation is correct in that we must read
> > data available. However, if the RESPONDING flag is set
> > and the URB submitted, we are already doing so.
>
> Sounds reasonable. Except that the bug proves we didn't.
>
> If you are right that service_outstanding_interrupt can race againts
> itself (and I don't doubt that), then I guess this could also happen
> between failure to submit the URB and clearing the flag?
>
> >> As for the XMM behaviour: it's been a long time since I tried any of
> >> those, but AFAIR one the major differences compared to Qualcomm was the
> >> strict queue handling in the firmware. This caused a number of problems
> >> where the cdc-wdm driver wanted to skip a message for some reason. So
> >> I'm not surprised that a bug like this is triggered by one of those
> >> modems. That's probably the only thing they are good for :-)
> >
> > I am not sure where exactly the issue lies here. Suggestions for
> > debugging?
>
> Nothing more than the obvious; Get one of those modems and do some
> usbmon snooping.
So for now, should we just revert this problem commit?
thanks,
greg k-h
next prev parent reply other threads:[~2024-04-15 9:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240314115132.3907-1-oneukum () suse ! com>
2024-04-15 5:42 ` [PATCH] usb: cdc-wdm: close race between read and workqueue Aleksander Morgado
2024-04-15 6:47 ` Bjørn Mork
2024-04-15 9:06 ` Oliver Neukum
2024-04-15 9:26 ` Bjørn Mork
2024-04-15 9:52 ` Greg KH [this message]
2024-04-15 10:07 ` Oliver Neukum
2024-04-15 10:14 ` Oliver Neukum
2024-04-15 10:53 ` Bjørn Mork
2024-04-15 11:08 ` Oliver Neukum
2024-04-15 8:42 ` Oliver Neukum
2024-04-15 12:08 ` Aleksander Morgado
2024-03-14 11:50 Oliver Neukum
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=2024041536-simile-flatly-824a@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=aleksandermj@chromium.org \
--cc=bjorn@mork.no \
--cc=ejcaruso@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=oneukum@suse.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox