All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Oliver Neukum <oneukum@suse.com>
Cc: Aleksander Morgado <aleksandermj@chromium.org>,
	linux-usb@vger.kernel.org, gregkh@linuxfoundation.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:26:00 +0200	[thread overview]
Message-ID: <878r1fht93.fsf@miraculix.mork.no> (raw)
In-Reply-To: <a6ad34d3-9cce-4178-8271-0e09ced2b6f4@suse.com> (Oliver Neukum's message of "Mon, 15 Apr 2024 11:06:33 +0200")

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.


Bjørn

  reply	other threads:[~2024-04-15  9:26 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 [this message]
2024-04-15  9:52         ` Greg KH
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=878r1fht93.fsf@miraculix.mork.no \
    --to=bjorn@mork.no \
    --cc=aleksandermj@chromium.org \
    --cc=ejcaruso@chromium.org \
    --cc=gregkh@linuxfoundation.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.