From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S970252AbeEXOAa (ORCPT ); Thu, 24 May 2018 10:00:30 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:42814 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965515AbeEXOA1 (ORCPT ); Thu, 24 May 2018 10:00:27 -0400 X-Google-Smtp-Source: AB8JxZrNIrGKM8FLtVljJD+zKBZel/6Ei2fIeWGaiHbnqOeOunbkk0FfDbtPc2Zrz1BK4tsoPbAuIg== Date: Thu, 24 May 2018 22:00:21 +0800 From: Martin Liu To: Alan Stern Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, jenhaochen@google.com, liumartin@google.com Subject: Re: [RFC] driver core: don't hold dev's parent lock when using async probe Message-ID: <20180524140021.GA214888@google.com> References: <20180522141227.GA118442@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 22, 2018 at 01:09:44PM -0400, Alan Stern wrote: > On Tue, 22 May 2018, martin_liu wrote: > > > not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the > > device's parent's lock during probe and remove")' since it has > > been there over 10 years. If we still need it and hard to fix it > > , the simple way is to find a place not to allow USB subsystem > > drivers to have async probe capability. Any suggestion is welcome. > > I don't think the "allows_async_probing" attribute is the best way to > attack this. Some other approach, like a special-purpose flag, might > be better. > > Yes, USB still needs to have parent's locks held during probing. > Here's the reason. A USB device can have multiple interfaces, each > bound to its own driver. A driver may sometimes need to issue a reset, > but in USB there's no way to reset a single interface. Only the entire > device can be reset, and of course this affects all the interfaces. > Therefore a driver needs to acquire the device lock before it can issue > a reset. > > The problem is that the driver's thread may already hold the device > lock. During a normal probe sequence, for example, the interfaces get > probed by the hub driver while it owns the device lock. But for probes > under other circumstances (for example, if the user writes to the > driver's "bind" attribute in sysfs), the device lock might not be held. > > A driver cannot tell these two cases apart. The only way to make it > work all the time is to have the caller _always_ hold the device lock > while the driver is probed (or the removed, for that matter). > > Alan Stern Thanks for the reply and more detail about the backgroud. I'd like to have a conclusion about it. Please kindly correct me if my understanding is wrong. Regarding to the "special-purpose flag", do you mean we could find a place in USB subsystem to have the flag set (not sure if it's easy to find it). Driver core would be base on the flag to decide if we need to hold the device's parent's lock. Martin 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: [RFC] driver core: don't hold dev's parent lock when using async probe From: martin_liu Message-Id: <20180524140021.GA214888@google.com> Date: Thu, 24 May 2018 22:00:21 +0800 To: Alan Stern Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, jenhaochen@google.com, liumartin@google.com List-ID: T24gVHVlLCBNYXkgMjIsIDIwMTggYXQgMDE6MDk6NDRQTSAtMDQwMCwgQWxhbiBTdGVybiB3cm90 ZToKPiBPbiBUdWUsIDIyIE1heSAyMDE4LCBtYXJ0aW5fbGl1IHdyb3RlOgo+IAo+ID4gbm90IHN1 cmUgaWYgd2Ugc3RpbGwgbmVlZCAnYmY3NGFkNWJjNDE3ICgiW1BBVENIXSBIb2xkIHRoZQo+ID4g ZGV2aWNlJ3MgcGFyZW50J3MgbG9jayBkdXJpbmcgcHJvYmUgYW5kIHJlbW92ZSIpJyBzaW5jZSBp dCBoYXMKPiA+IGJlZW4gdGhlcmUgb3ZlciAxMCB5ZWFycy4gSWYgd2Ugc3RpbGwgbmVlZCBpdCBh bmQgaGFyZCB0byBmaXggaXQKPiA+ICwgdGhlIHNpbXBsZSB3YXkgaXMgdG8gZmluZCBhIHBsYWNl IG5vdCB0byBhbGxvdyBVU0Igc3Vic3lzdGVtCj4gPiBkcml2ZXJzIHRvIGhhdmUgYXN5bmMgcHJv YmUgY2FwYWJpbGl0eS4gQW55IHN1Z2dlc3Rpb24gaXMgd2VsY29tZS4KPiAKPiBJIGRvbid0IHRo aW5rIHRoZSAiYWxsb3dzX2FzeW5jX3Byb2JpbmciIGF0dHJpYnV0ZSBpcyB0aGUgYmVzdCB3YXkg dG8gCj4gYXR0YWNrIHRoaXMuICBTb21lIG90aGVyIGFwcHJvYWNoLCBsaWtlIGEgc3BlY2lhbC1w dXJwb3NlIGZsYWcsIG1pZ2h0IAo+IGJlIGJldHRlci4KPiAKPiBZZXMsIFVTQiBzdGlsbCBuZWVk cyB0byBoYXZlIHBhcmVudCdzIGxvY2tzIGhlbGQgZHVyaW5nIHByb2JpbmcuICAKPiBIZXJlJ3Mg dGhlIHJlYXNvbi4gIEEgVVNCIGRldmljZSBjYW4gaGF2ZSBtdWx0aXBsZSBpbnRlcmZhY2VzLCBl YWNoCj4gYm91bmQgdG8gaXRzIG93biBkcml2ZXIuICBBIGRyaXZlciBtYXkgc29tZXRpbWVzIG5l ZWQgdG8gaXNzdWUgYSByZXNldCwKPiBidXQgaW4gVVNCIHRoZXJlJ3Mgbm8gd2F5IHRvIHJlc2V0 IGEgc2luZ2xlIGludGVyZmFjZS4gIE9ubHkgdGhlIGVudGlyZQo+IGRldmljZSBjYW4gYmUgcmVz ZXQsIGFuZCBvZiBjb3Vyc2UgdGhpcyBhZmZlY3RzIGFsbCB0aGUgaW50ZXJmYWNlcy4gIAo+IFRo ZXJlZm9yZSBhIGRyaXZlciBuZWVkcyB0byBhY3F1aXJlIHRoZSBkZXZpY2UgbG9jayBiZWZvcmUg aXQgY2FuIGlzc3VlCj4gYSByZXNldC4KPiAKPiBUaGUgcHJvYmxlbSBpcyB0aGF0IHRoZSBkcml2 ZXIncyB0aHJlYWQgbWF5IGFscmVhZHkgaG9sZCB0aGUgZGV2aWNlCj4gbG9jay4gIER1cmluZyBh IG5vcm1hbCBwcm9iZSBzZXF1ZW5jZSwgZm9yIGV4YW1wbGUsIHRoZSBpbnRlcmZhY2VzIGdldAo+ IHByb2JlZCBieSB0aGUgaHViIGRyaXZlciB3aGlsZSBpdCBvd25zIHRoZSBkZXZpY2UgbG9jay4g IEJ1dCBmb3IgcHJvYmVzCj4gdW5kZXIgb3RoZXIgY2lyY3Vtc3RhbmNlcyAoZm9yIGV4YW1wbGUs IGlmIHRoZSB1c2VyIHdyaXRlcyB0byB0aGUKPiBkcml2ZXIncyAiYmluZCIgYXR0cmlidXRlIGlu IHN5c2ZzKSwgdGhlIGRldmljZSBsb2NrIG1pZ2h0IG5vdCBiZSBoZWxkLgo+IAo+IEEgZHJpdmVy IGNhbm5vdCB0ZWxsIHRoZXNlIHR3byBjYXNlcyBhcGFydC4gIFRoZSBvbmx5IHdheSB0byBtYWtl IGl0Cj4gd29yayBhbGwgdGhlIHRpbWUgaXMgdG8gaGF2ZSB0aGUgY2FsbGVyIF9hbHdheXNfIGhv bGQgdGhlIGRldmljZSBsb2NrCj4gd2hpbGUgdGhlIGRyaXZlciBpcyBwcm9iZWQgKG9yIHRoZSBy ZW1vdmVkLCBmb3IgdGhhdCBtYXR0ZXIpLgo+IAo+IEFsYW4gU3Rlcm4KClRoYW5rcyBmb3IgdGhl IHJlcGx5IGFuZCBtb3JlIGRldGFpbCBhYm91dCB0aGUgYmFja2dyb3VkLiBJJ2QgbGlrZSB0bwpo YXZlIGEgY29uY2x1c2lvbiBhYm91dCBpdC4gUGxlYXNlIGtpbmRseSBjb3JyZWN0IG1lIGlmIG15 IHVuZGVyc3RhbmRpbmcKaXMgd3JvbmcuIFJlZ2FyZGluZyB0byB0aGUgInNwZWNpYWwtcHVycG9z ZSBmbGFnIiwgZG8geW91IG1lYW4gd2UgY291bGQKZmluZCBhIHBsYWNlIGluIFVTQiBzdWJzeXN0 ZW0gdG8gaGF2ZSB0aGUgZmxhZyBzZXQgKG5vdCBzdXJlIGlmIGl0J3MKZWFzeSB0byBmaW5kIGl0 KS4gRHJpdmVyIGNvcmUgd291bGQgYmUgYmFzZSBvbiB0aGUgZmxhZyB0byBkZWNpZGUgaWYgd2UK bmVlZCB0byBob2xkIHRoZSBkZXZpY2UncyBwYXJlbnQncyBsb2NrLgoKTWFydGluCi0tLQpUbyB1 bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGlu dXgtdXNiIiBpbgp0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVs Lm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9y ZG9tby1pbmZvLmh0bWwK