Message-ID: <ee8203fd-49ca-4106-b8c3-4397c933de46@rowland.harvard.edu>
Date: Wed, 16 Jul 2025 10:38:06 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: Sean Young <sean@...s.org>
Cc: Hillf Danton <hdanton@...a.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	syzbot+592e2ab8775dbe0bf09a@...kaller.appspotmail.com,
	LKML <linux-kernel@...r.kernel.org>,
	Mauro Carvalho Chehab <mchehab@...nel.org>
Subject: Re: [PATCH] media: imon: make send_packet() more robust

On Wed, Jul 16, 2025 at 10:38:23AM +0100, Sean Young wrote:
> On Tue, Jul 15, 2025 at 09:30:02PM -0400, Alan Stern wrote:
> > On Tue, Jul 15, 2025 at 09:19:18PM +0100, Sean Young wrote:
> > > Hi Alan,
> > > 
> > > On Sun, Jul 13, 2025 at 11:21:24AM -0400, Alan Stern wrote:
> > > > On Sun, Jul 13, 2025 at 04:11:47PM +0800, Hillf Danton wrote:
> > > > > [loop Alan in]
> > > > 
> > > > I assume you're interested in the question of when to avoid resubmitting 
> > > > URBs.
> > 
> > > > In theory it's okay to resubmit _if_ the driver has a robust 
> > > > error-recovery scheme (such as giving up after some fixed limit on the 
> > > > number of errors or after some fixed time has elapsed, perhaps with a 
> > > > time delay to prevent a flood of errors).  Most drivers don't bother to 
> > > > do this; they simply give up right away.  This makes them more 
> > > > vulnerable to short-term noise interference during USB transfers, but in 
> > > > reality such interference is quite rare.  There's nothing really wrong 
> > > > with giving up right away.
> > > > 
> > > > As to which error codes drivers should pay attention to...  In most 
> > > > cases they only look at -EPROTO.  According to 
> > > > Documentation/driver-api/usb/error-codes.rst, -EILSEQ and -ETIME are 
> > > > also possible errors when a device has been unplugged, so it wouldn't 
> > > > hurt to check for them too.  But most host controller drivers don't 
> > > > bother to issue them; -EPROTO is by far the most common error code 
> > > > following an unplug.
> > > 
> > > Thank you for explaining that, very helpful. Would it be useful to have
> > > this in the USB completion handler documentation?
> > 
> > I don't know what USB completion handler documentation you're talking 
> > about.  Is it something in the Documentation/ directory?  If it is then 
> > it should already include or refer to error-codes.rst.
> 
> I can't see anything in error-codes.rst or URB.rst about the possibility
> of retrying after -EPROTO errors or how the callback should respond if
> it wants to give up. USB drivers seem to do all manner of different things.

error-codes.rst is meant merely to explain the meanings of the different 
error codes.  How to deal with them when they occur is not in its scope.

Drivers behave in different ways because they were written by different 
people and serve different purposes.  For example, data loss in a 
mass-storage driver would be a lot more serious than data loss in a 
mouse driver, so of course the two drivers don't use the same amount of 
care when recovering from errors.

As for how a callback should be behave if it wants to give up, that 
depends on how the driver is designed.  There is no one single answer 
appropriate for all drivers.  In the simplest case, where the driver 
always keeps one URB in flight and resubmits the URB whenever it 
completes, giving up is easy -- just don't resubmit the URB!  This will 
immediately end all the communication with the device.

> > > I think that is why this driver code is so awkward.
> > 
> > That's what usb_driver_claim_interface() is for.  IIRC, the cdc-acm 
> > driver uses it in exactly this way.
> 
> Very interesting, we should look at re-writing this driver. Note this
> function is not documented in Documentation/driver-api/usb/

The documentation files are quite old and were never complete.  Nowadays 
we rely much more heavily on the kerneldoc in the source code itself.

> Thank you for your help

You're welcome.

Alan Stern

Powered by blists - more mailing lists