From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FA7CC43381 for ; Fri, 29 Mar 2019 14:13:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 768D0206C0 for ; Fri, 29 Mar 2019 14:13:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729378AbfC2ON6 (ORCPT ); Fri, 29 Mar 2019 10:13:58 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:49076 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1728833AbfC2ON5 (ORCPT ); Fri, 29 Mar 2019 10:13:57 -0400 Received: (qmail 2706 invoked by uid 2102); 29 Mar 2019 10:13:56 -0400 Received: from localhost (sendmail-bs@127.0.0.1) by localhost with SMTP; 29 Mar 2019 10:13:56 -0400 Date: Fri, 29 Mar 2019 10:13:56 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Oliver Neukum cc: gregkh@linuxfoundation.org, , , , , , Subject: Re: [PATCH] usb: uas: fix usb subsystem hang after power off hub port In-Reply-To: <1553791752.6310.2.camel@suse.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 28 Mar 2019, Oliver Neukum wrote: > 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. Maybe I don't understand the problem correctly. It sounds like the problem occurs because uas tries to rebind (Where, how, and why does it do this? I don't see anything special in uas_post_reset) using the same scsi_host structure as before instead of creating a new one. Trying to reuse a defunct host is definitely not allowed. But if uas didn't try to do anything special, it would be unbound from the device, causing the scsi_host to be destroyed. Then when usbcore automatically attempted a rebind, uas would create a new scsi_host and the rebind would have a chance to succeed, without hanging anything. If this description is wrong, please explain more fully. Alan Stern 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: Alan Stern Message-Id: Date: Fri, 29 Mar 2019 10:13:56 -0400 (EDT) To: Oliver Neukum 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: T24gVGh1LCAyOCBNYXIgMjAxOSwgT2xpdmVyIE5ldWt1bSB3cm90ZToKCj4gT24gRG8sIDIwMTkt MDMtMjggYXQgMTE6NTcgLTA0MDAsIEFsYW4gU3Rlcm4gd3JvdGU6Cj4gPiBPbiBUaHUsIDI4IE1h ciAyMDE5LCBPbGl2ZXIgTmV1a3VtIHdyb3RlOgo+ID4gCj4gPiA+IE9uIERvLCAyMDE5LTAzLTI4 IGF0IDA3OjUzICswMDAwLCBLZW50by5BLktvYmF5YXNoaUBzb255LmNvbSB3cm90ZToKPiA+ID4g PiBIaSwKPiA+ID4gPiAKPiA+ID4gPiA+IFNvcnJ5LAo+ID4gPiA+ID4gCj4gPiA+ID4gPiBJIHRo b3VnaHQgdGhpcyB3YXMgY2xlYXIuIFlvdXIgcGF0Y2ggaXMgbWFraW5nIHRoZSBhc3N1bXB0aW9u IHRoYXQgdGhlIHJlc2V0IGlzIHRyaWdnZXJlZCBieSB0aGUgU0NTSSBsYXllci4gWW91IGNhbm5v dCBtYWtlIHRoYXQgYXNzdW1wdGlvbiwgYXMgdGhlcmUgaXMgYW4gaW9jdGwgZm9yIHJlc2V0dGlu ZyBhIFVTQiBkZXZpY2UuCj4gPiA+ID4gPiBJbiBjYXNlIHdlIGFyZSBnZXR0aW5nIGFuIGVycm9y IGR1cmluZyB0aGUgcmVzZXQgKG91ciBlbmRwb2ludHMgdmFuaXNoKSwgdGhlIGRldmljZSBkcml2 ZXIgbXVzdCByZXBvcnQgdGhhdCB0byB0aGUgVVNCIGxheWVyLCBzbyB0aGUgZHJpdmVyIHdpbGwg YWx3YXlzIGJlIGRpc2Nvbm5lY3RlZC4KPiA+ID4gPiA+IFdlIGNhbm5vdCBkcm9wIGVycm9ycy4K PiA+ID4gPiA+IAo+ID4gPiA+ID4gICBSZWdhcmRzCj4gPiA+ID4gPiAgICAgICAgICAgT2xpdmVy Cj4gPiA+ID4gCj4gPiA+ID4gVGhpcyBwYXRjaCBtb2RpZmllZCB1YXNfcG9zdF9yZXNldCB0byBz a2lwIHJlYmluZCBvcGVyYXRpb24gdG8gYXZvaWQgZXhjZXB0aW9uIHdoaWxlIC1FTk9ERVYgaGFw cGVucyBub3QgZHJvcCBlcnJvci4KPiA+ID4gPiBJZiB1YXNfcG9zdF9yZXNldCBoYXBwZW5zIC1F Tk9ERVYsIHVzYl9yZXNldF9hbmRfdmVyaWZ5X2RldmljZSBtdXN0IGhhcHBlbiBlcnJvci4KPiA+ ID4gPiBTbyx3aGVuIHdlIHVzZSBpb2N0bChVU0JERVZGU19SRVNFVCkgdG8gcmVzZXQgZGV2aWNl LCBpZiB1c2JfcmVzZXRfYW5kX3ZlcmlmeV9kZXZpY2UgaGFwcGVucyBlcnJvciwgdGhlIGVycm9y IHdpbGwgYmUgcmVwb3J0ZWQgdGhyb3VnaCBpb2N0bCByZXR1cm4gdmFsdWUuIAo+ID4gPiAKPiA+ ID4gT0ssIEl0IGlzIHBvc3NpYmxlIHRoYXQgSSBhbSBzdHVwaWQuIFdlIG11c3QgcmViaW5kIGlm IHVhc19wb3N0X3Jlc2V0KCkKPiA+ID4gZmFpbHMuIFRoZSBkcml2ZXIgd2lsbCBjcmFzaCB3aXRo b3V0IHRoZSBlbmRwb2ludHMuIENhbiB5b3UgcGxlYXNlCj4gPiA+IGV4cGxhaW4gYWdhaW4gaW4g Z3JlYXRlciBkZXRhaWwsIHdoYXQgeW91IGFyZSB0cnlpbmcgdG8gYWNoaWV2ZT8KPiA+IAo+ID4g QWN0dWFsbHkgbm8sIHRoZSBkcml2ZXIgZG9lc24ndCBoYXZlIHRvIGRvIGFueXRoaW5nIGlmIHRo ZSBwb3N0LXJlc2V0IAo+ID4gbWV0aG9kIGZhaWxzLiAgdXNiY29yZSB3aWxsIHRha2UgY2FyZSBv ZiByZWJpbmRpbmcgYXV0b21hdGljYWxseS4KPiAKPiBZZXMsIGJ1dCB0aGUgcmViaW5kaW5nIGlz IG5vdCBvcHRpb25hbC4gSGVuY2UsIGVpdGhlciB0aGUgcG9zdF9yZXNldCgpCj4gbXVzdCBmYWls IHRvIHRyaWdnZXIgaXQsIG9yIGl0IHdpbGwgYmUgdHJpZ2dlcmVkIGFueXdheS4KPiBTbyBpZiB0 aGUgcmViaW5kaW5nIGhhbmdzIHRoZSBtYWNoaW5lLCBJIGNhbm5vdCBzZWUgaG93IGFsdGVyCj4g Y2hhbmdpbmcgdGhlIHJldHVybiB2YWx1ZSBvZiB1YXNfcG9zdF9yZXNldCgpIHdvdWxkIGhlbHAu Cj4gCj4gSXQgbG9va3MgdG8gbWUgbGlrZSB0aGUgc3RhdGUgdHJhbnNpdGlvbnMgb2YgU0NTSSBh cmUgdG9vCj4gcmVzdHJpY3RpdmUuCgpNYXliZSBJIGRvbid0IHVuZGVyc3RhbmQgdGhlIHByb2Js ZW0gY29ycmVjdGx5LiAgSXQgc291bmRzIGxpa2UgdGhlCnByb2JsZW0gb2NjdXJzIGJlY2F1c2Ug dWFzIHRyaWVzIHRvIHJlYmluZCAoV2hlcmUsIGhvdywgYW5kIHdoeSBkb2VzIGl0CmRvIHRoaXM/ ICBJIGRvbid0IHNlZSBhbnl0aGluZyBzcGVjaWFsIGluIHVhc19wb3N0X3Jlc2V0KSB1c2luZyB0 aGUKc2FtZSBzY3NpX2hvc3Qgc3RydWN0dXJlIGFzIGJlZm9yZSBpbnN0ZWFkIG9mIGNyZWF0aW5n IGEgbmV3IG9uZS4gIApUcnlpbmcgdG8gcmV1c2UgYSBkZWZ1bmN0IGhvc3QgaXMgZGVmaW5pdGVs eSBub3QgYWxsb3dlZC4KCkJ1dCBpZiB1YXMgZGlkbid0IHRyeSB0byBkbyBhbnl0aGluZyBzcGVj aWFsLCBpdCB3b3VsZCBiZSB1bmJvdW5kIGZyb20KdGhlIGRldmljZSwgY2F1c2luZyB0aGUgc2Nz aV9ob3N0IHRvIGJlIGRlc3Ryb3llZC4gIFRoZW4gd2hlbiB1c2Jjb3JlCmF1dG9tYXRpY2FsbHkg YXR0ZW1wdGVkIGEgcmViaW5kLCB1YXMgd291bGQgY3JlYXRlIGEgbmV3IHNjc2lfaG9zdCBhbmQK dGhlIHJlYmluZCB3b3VsZCBoYXZlIGEgY2hhbmNlIHRvIHN1Y2NlZWQsIHdpdGhvdXQgaGFuZ2lu ZyBhbnl0aGluZy4KCklmIHRoaXMgZGVzY3JpcHRpb24gaXMgd3JvbmcsIHBsZWFzZSBleHBsYWlu IG1vcmUgZnVsbHkuCgpBbGFuIFN0ZXJuCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Stern Subject: Re: [PATCH] usb: uas: fix usb subsystem hang after power off hub port Date: Fri, 29 Mar 2019 10:13:56 -0400 (EDT) Message-ID: References: <1553791752.6310.2.camel@suse.com> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Return-path: In-Reply-To: <1553791752.6310.2.camel@suse.com> Sender: linux-kernel-owner@vger.kernel.org To: Oliver Neukum 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 Thu, 28 Mar 2019, Oliver Neukum wrote: > 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. Maybe I don't understand the problem correctly. It sounds like the problem occurs because uas tries to rebind (Where, how, and why does it do this? I don't see anything special in uas_post_reset) using the same scsi_host structure as before instead of creating a new one. Trying to reuse a defunct host is definitely not allowed. But if uas didn't try to do anything special, it would be unbound from the device, causing the scsi_host to be destroyed. Then when usbcore automatically attempted a rebind, uas would create a new scsi_host and the rebind would have a chance to succeed, without hanging anything. If this description is wrong, please explain more fully. Alan Stern