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 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.