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.

quoted

       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