Am Samstag, den 23.01.2021, 11:57 +0900 schrieb Tetsuo Handa:
> On 2021/01/22 0:30, Oliver Neukum wrote:

Hi,

> Right. Shouldn't remaining
> 
>   kill_urbs(desc);
>   cancel_work_sync(&desc->rxwork);
>   cancel_work_sync(&desc->service_outs_intr);
> 
> sequence in wdm_suspend() and wdm_pre_reset() be updated as well?

Yes, they should.

> >        Unfortunately we have in wdm_in_callback() the following code path
> > 
> >         if (desc->rerr) {
> >                 /*
> >                  * Since there was an error, userspace may decide to not read
> >                  * any data after poll'ing.
> >                  * We should respond to further attempts from the device to send
> >                  * data, so that we can get unstuck.
> >                  */
> >                 schedule_work(&desc->service_outs_intr);
> > 
> > It looks to me like we have a circular dependency here and this needs some
> > change to break. What do you think about the attached patch?
> 
> I don't know how poisoning works. But why can't we simply use test_bit() on

It makes subsequent usb_submit_urb() fail.

> WDM_SUSPENDING/WDM_RESETTING/WDM_DISCONNECTING flags, for schedule_work() in
> wdm_in_callback() is called with desc->iuspin (which serializes setting of
> these flags) held.

In theory this could be done, yet that would take three additional
tests as opposed to the test for poisoning that usbcore does anyway.

> By the way, since someone might interpret "broken" as "out of order / not working",
> I expect not using "This needs to be broken." in the commit message. There would be
> some better idiom.

Right. I changed the message.

Could you test whether the attached patch fixes your issue?

	Regards
		Oliver