Re: Re: Re: Re: Re: [syzbot] INFO: rcu detected stall in tx

[dave penkler]

On Thu, 6 May 2021 at 22:31, Guido Kiener
<Guido.Kiener@xxxxxxxxxxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: Alan Stern
> > Sent: Thursday, May 6, 2021 8:32 PM
> > To: Kiener Guido 14DS1
> >
> > On Thu, May 06, 2021 at 05:44:55PM +0000, Guido Kiener wrote:
> > > > -----Original Message-----
> > > > From: Alan Stern
> > > > Sent: Thursday, May 6, 2021 3:49 PM
> > > > To: Kiener Guido 14DS1 <Guido.Kiener@xxxxxxxxxxxxxxxxx>
> > > > >
> > > > > Thanks for your assessment. I agree with the general feeling. I
> > > > > counted about hundred specific usb drivers, so wouldn't it be
> > > > > better to fix the
> > > > problem in some of the host drivers (e.g. urb.c)?
> > > > > We could return an error when calling usb_submit_urb() on an erroneous
> > pipe.
> > > > > I cannot estimate the side effects and we need to check all
> > > > > drivers again how they deal with the error situation. Maybe there
> > > > > are some special driver
> > > > that need a specialized error handling.
> > > > > In this case these drivers could reset the (new?) error flag to
> > > > > allow calling usb_submit_urb() again without error. This could work, isn't it?
> > > >
> > > > That is feasible, although it would be an awkward approach.  As you
> > > > said, the side effects aren't clear.  But it might work.
> > >
> > > Otherwise I see only the other approach to change hundred drivers and
> > > add the cases EPROTO, EILSEQ and ETIME in each callback handler. The
> > > usbtmc driver already respects the EILSEQ and ETIME, and only EPROTO is
> > missing.
> > > The rest should be more a management task.
> > > BTW do you assume it is only a problem for INT pipes or is it also a
> > > problem for isochronous and bulk transfers?
> >
> > All of them.  Control too.
> >
> > > > Will you be able to test patches?
> > >
> > > I only can test the USBTMC function in some different PCs. I do not
> > > have automated regression tests for USB drivers or Linux kernels.
> > > Maybe there is company who could do that.
> >
> > Well then, if I do find time to write a patch, I'll ask you to try it out with the usbtmc
> > driver.
>
> You mean that you will do a patch in urb.c or a host driver? Or just add a line in usbtmc.c?
> Anyhow there is no hurry. On May 20 I will send you a mail if I'm able to
> provoke one of these hardware errors EPROTO, EILSQ, or ETIME. Otherwise
> it doesn't make sense to test it.
>
> -Guido

EPROTO is a link level issue and needs to be handled by the host driver.
When the host driver detects a protocol error while processing an URB
it completes the URB with EPROTO status and marks the endpoint as
halted.
When the class driver resubmits the URB and the if the host driver
finds the endpoint still marked as halted it should return EPIPE
status on the resubmitted URB
When the class driver and usbtmc in particular receives an URB with
EPIPE status it cleans up and does not resubmit.
Can someone from syzbot land please confirm whether usbtmc running on
the xhci host driver causes an RCU stall to be detected ?
-Dave