From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oliver Neukum Subject: Re: [PATCH] usb: uas: fix usb subsystem hang after power off hub port Date: Thu, 28 Mar 2019 17:49:12 +0100 Message-ID: <1553791752.6310.2.camel@suse.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Alan Stern Cc: gregkh@linuxfoundation.org, usb-storage@lists.one-eyed-alien.net, Jacky.Cao@sony.com, Kento.A.Kobayashi@sony.com, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, linux-usb@vger.kernel.org List-Id: linux-scsi@vger.kernel.org On Do, 2019-03-28 at 11:57 -0400, Alan Stern wrote: > On Thu, 28 Mar 2019, Oliver Neukum wrote: > > > On Do, 2019-03-28 at 07:53 +0000, Kento.A.Kobayashi@sony.com wrote: > > > Hi, > > > > > > > Sorry, > > > > > > > > I thought this was clear. Your patch is making the assumption that the reset is triggered by the SCSI layer. You cannot make that assumption, as there is an ioctl for resetting a USB device. > > > > In case we are getting an error during the reset (our endpoints vanish), the device driver must report that to the USB layer, so the driver will always be disconnected. > > > > We cannot drop errors. > > > > > > > > Regards > > > > Oliver > > > > > > This patch modified uas_post_reset to skip rebind operation to avoid exception while -ENODEV happens not drop error. > > > If uas_post_reset happens -ENODEV, usb_reset_and_verify_device must happen error. > > > So,when we use ioctl(USBDEVFS_RESET) to reset device, if usb_reset_and_verify_device happens error, the error will be reported through ioctl return value. > > > > OK, It is possible that I am stupid. We must rebind if uas_post_reset() > > fails. The driver will crash without the endpoints. Can you please > > explain again in greater detail, what you are trying to achieve? > > Actually no, the driver doesn't have to do anything if the post-reset > method fails. usbcore will take care of rebinding automatically. Yes, but the rebinding is not optional. Hence, either the post_reset() must fail to trigger it, or it will be triggered anyway. So if the rebinding hangs the machine, I cannot see how alter changing the return value of uas_post_reset() would help. It looks to me like the state transitions of SCSI are too restrictive. Regards Oliver From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb: uas: fix usb subsystem hang after power off hub port From: Oliver Neukum Message-Id: <1553791752.6310.2.camel@suse.com> Date: Thu, 28 Mar 2019 17:49:12 +0100 To: Alan Stern Cc: gregkh@linuxfoundation.org, usb-storage@lists.one-eyed-alien.net, Jacky.Cao@sony.com, Kento.A.Kobayashi@sony.com, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, linux-usb@vger.kernel.org List-ID: T24gRG8sIDIwMTktMDMtMjggYXQgMTE6NTcgLTA0MDAsIEFsYW4gU3Rlcm4gd3JvdGU6Cj4gT24g VGh1LCAyOCBNYXIgMjAxOSwgT2xpdmVyIE5ldWt1bSB3cm90ZToKPiAKPiA+IE9uIERvLCAyMDE5 LTAzLTI4IGF0IDA3OjUzICswMDAwLCBLZW50by5BLktvYmF5YXNoaUBzb255LmNvbSB3cm90ZToK PiA+ID4gSGksCj4gPiA+IAo+ID4gPiA+IFNvcnJ5LAo+ID4gPiA+IAo+ID4gPiA+IEkgdGhvdWdo dCB0aGlzIHdhcyBjbGVhci4gWW91ciBwYXRjaCBpcyBtYWtpbmcgdGhlIGFzc3VtcHRpb24gdGhh dCB0aGUgcmVzZXQgaXMgdHJpZ2dlcmVkIGJ5IHRoZSBTQ1NJIGxheWVyLiBZb3UgY2Fubm90IG1h a2UgdGhhdCBhc3N1bXB0aW9uLCBhcyB0aGVyZSBpcyBhbiBpb2N0bCBmb3IgcmVzZXR0aW5nIGEg VVNCIGRldmljZS4KPiA+ID4gPiBJbiBjYXNlIHdlIGFyZSBnZXR0aW5nIGFuIGVycm9yIGR1cmlu ZyB0aGUgcmVzZXQgKG91ciBlbmRwb2ludHMgdmFuaXNoKSwgdGhlIGRldmljZSBkcml2ZXIgbXVz dCByZXBvcnQgdGhhdCB0byB0aGUgVVNCIGxheWVyLCBzbyB0aGUgZHJpdmVyIHdpbGwgYWx3YXlz IGJlIGRpc2Nvbm5lY3RlZC4KPiA+ID4gPiBXZSBjYW5ub3QgZHJvcCBlcnJvcnMuCj4gPiA+ID4g Cj4gPiA+ID4gICBSZWdhcmRzCj4gPiA+ID4gICAgICAgICAgIE9saXZlcgo+ID4gPiAKPiA+ID4g VGhpcyBwYXRjaCBtb2RpZmllZCB1YXNfcG9zdF9yZXNldCB0byBza2lwIHJlYmluZCBvcGVyYXRp b24gdG8gYXZvaWQgZXhjZXB0aW9uIHdoaWxlIC1FTk9ERVYgaGFwcGVucyBub3QgZHJvcCBlcnJv ci4KPiA+ID4gSWYgdWFzX3Bvc3RfcmVzZXQgaGFwcGVucyAtRU5PREVWLCB1c2JfcmVzZXRfYW5k X3ZlcmlmeV9kZXZpY2UgbXVzdCBoYXBwZW4gZXJyb3IuCj4gPiA+IFNvLHdoZW4gd2UgdXNlIGlv Y3RsKFVTQkRFVkZTX1JFU0VUKSB0byByZXNldCBkZXZpY2UsIGlmIHVzYl9yZXNldF9hbmRfdmVy aWZ5X2RldmljZSBoYXBwZW5zIGVycm9yLCB0aGUgZXJyb3Igd2lsbCBiZSByZXBvcnRlZCB0aHJv dWdoIGlvY3RsIHJldHVybiB2YWx1ZS4gCj4gPiAKPiA+IE9LLCBJdCBpcyBwb3NzaWJsZSB0aGF0 IEkgYW0gc3R1cGlkLiBXZSBtdXN0IHJlYmluZCBpZiB1YXNfcG9zdF9yZXNldCgpCj4gPiBmYWls cy4gVGhlIGRyaXZlciB3aWxsIGNyYXNoIHdpdGhvdXQgdGhlIGVuZHBvaW50cy4gQ2FuIHlvdSBw bGVhc2UKPiA+IGV4cGxhaW4gYWdhaW4gaW4gZ3JlYXRlciBkZXRhaWwsIHdoYXQgeW91IGFyZSB0 cnlpbmcgdG8gYWNoaWV2ZT8KPiAKPiBBY3R1YWxseSBubywgdGhlIGRyaXZlciBkb2Vzbid0IGhh dmUgdG8gZG8gYW55dGhpbmcgaWYgdGhlIHBvc3QtcmVzZXQgCj4gbWV0aG9kIGZhaWxzLiAgdXNi Y29yZSB3aWxsIHRha2UgY2FyZSBvZiByZWJpbmRpbmcgYXV0b21hdGljYWxseS4KClllcywgYnV0 IHRoZSByZWJpbmRpbmcgaXMgbm90IG9wdGlvbmFsLiBIZW5jZSwgZWl0aGVyIHRoZSBwb3N0X3Jl c2V0KCkKbXVzdCBmYWlsIHRvIHRyaWdnZXIgaXQsIG9yIGl0IHdpbGwgYmUgdHJpZ2dlcmVkIGFu eXdheS4KU28gaWYgdGhlIHJlYmluZGluZyBoYW5ncyB0aGUgbWFjaGluZSwgSSBjYW5ub3Qgc2Vl IGhvdyBhbHRlcgpjaGFuZ2luZyB0aGUgcmV0dXJuIHZhbHVlIG9mIHVhc19wb3N0X3Jlc2V0KCkg d291bGQgaGVscC4KCkl0IGxvb2tzIHRvIG1lIGxpa2UgdGhlIHN0YXRlIHRyYW5zaXRpb25zIG9m IFNDU0kgYXJlIHRvbwpyZXN0cmljdGl2ZS4KCglSZWdhcmRzCgkJT2xpdmVyCg==