From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1033464AbeEXQFl (ORCPT ); Thu, 24 May 2018 12:05:41 -0400 Received: from mail-pg0-f66.google.com ([74.125.83.66]:39308 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030546AbeEXQFg (ORCPT ); Thu, 24 May 2018 12:05:36 -0400 X-Google-Smtp-Source: AB8JxZoyAC8D3EeRMUD1TD/u39jP4ui0zx7CYZLaUoTHsexh5FGbjURMnlLpjYbd14tO9TPJwF27ww== Date: Fri, 25 May 2018 00:05:32 +0800 From: Martin Liu To: Alan Stern Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, jenhaochen@google.com Subject: Re: [RFC] driver core: don't hold dev's parent lock when using async probe Message-ID: <20180524160532.GA4803@google.com> References: <20180524140021.GA214888@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 Thu, May 24, 2018 at 11:02:57AM -0400, Alan Stern wrote: > On Thu, 24 May 2018, Martin Liu wrote: > > > 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. > > Yes, except that the flag would not be in the USB subsystem. It would > be in the device, device_type, or bus_type structure, so that the > driver core could access it. > > Alan Stern Thanks for the quick feedback and the suggestion. will try to figure out how it works. 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: <20180524160532.GA4803@google.com> Date: Fri, 25 May 2018 00:05:32 +0800 To: Alan Stern Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, jenhaochen@google.com List-ID: T24gVGh1LCBNYXkgMjQsIDIwMTggYXQgMTE6MDI6NTdBTSAtMDQwMCwgQWxhbiBTdGVybiB3cm90 ZToKPiBPbiBUaHUsIDI0IE1heSAyMDE4LCBNYXJ0aW4gTGl1IHdyb3RlOgo+IAo+ID4gT24gVHVl LCBNYXkgMjIsIDIwMTggYXQgMDE6MDk6NDRQTSAtMDQwMCwgQWxhbiBTdGVybiB3cm90ZToKPiA+ ID4gT24gVHVlLCAyMiBNYXkgMjAxOCwgbWFydGluX2xpdSB3cm90ZToKPiA+ID4gCj4gPiA+ID4g bm90IHN1cmUgaWYgd2Ugc3RpbGwgbmVlZCAnYmY3NGFkNWJjNDE3ICgiW1BBVENIXSBIb2xkIHRo ZQo+ID4gPiA+IGRldmljZSdzIHBhcmVudCdzIGxvY2sgZHVyaW5nIHByb2JlIGFuZCByZW1vdmUi KScgc2luY2UgaXQgaGFzCj4gPiA+ID4gYmVlbiB0aGVyZSBvdmVyIDEwIHllYXJzLiBJZiB3ZSBz dGlsbCBuZWVkIGl0IGFuZCBoYXJkIHRvIGZpeCBpdAo+ID4gPiA+ICwgdGhlIHNpbXBsZSB3YXkg aXMgdG8gZmluZCBhIHBsYWNlIG5vdCB0byBhbGxvdyBVU0Igc3Vic3lzdGVtCj4gPiA+ID4gZHJp dmVycyB0byBoYXZlIGFzeW5jIHByb2JlIGNhcGFiaWxpdHkuIEFueSBzdWdnZXN0aW9uIGlzIHdl bGNvbWUuCj4gPiA+IAo+ID4gPiBJIGRvbid0IHRoaW5rIHRoZSAiYWxsb3dzX2FzeW5jX3Byb2Jp bmciIGF0dHJpYnV0ZSBpcyB0aGUgYmVzdCB3YXkgdG8gCj4gPiA+IGF0dGFjayB0aGlzLiAgU29t ZSBvdGhlciBhcHByb2FjaCwgbGlrZSBhIHNwZWNpYWwtcHVycG9zZSBmbGFnLCBtaWdodCAKPiA+ ID4gYmUgYmV0dGVyLgo+ID4gPiAKPiA+ID4gWWVzLCBVU0Igc3RpbGwgbmVlZHMgdG8gaGF2ZSBw YXJlbnQncyBsb2NrcyBoZWxkIGR1cmluZyBwcm9iaW5nLiAgCj4gPiA+IEhlcmUncyB0aGUgcmVh c29uLiAgQSBVU0IgZGV2aWNlIGNhbiBoYXZlIG11bHRpcGxlIGludGVyZmFjZXMsIGVhY2gKPiA+ ID4gYm91bmQgdG8gaXRzIG93biBkcml2ZXIuICBBIGRyaXZlciBtYXkgc29tZXRpbWVzIG5lZWQg dG8gaXNzdWUgYSByZXNldCwKPiA+ID4gYnV0IGluIFVTQiB0aGVyZSdzIG5vIHdheSB0byByZXNl dCBhIHNpbmdsZSBpbnRlcmZhY2UuICBPbmx5IHRoZSBlbnRpcmUKPiA+ID4gZGV2aWNlIGNhbiBi ZSByZXNldCwgYW5kIG9mIGNvdXJzZSB0aGlzIGFmZmVjdHMgYWxsIHRoZSBpbnRlcmZhY2VzLiAg Cj4gPiA+IFRoZXJlZm9yZSBhIGRyaXZlciBuZWVkcyB0byBhY3F1aXJlIHRoZSBkZXZpY2UgbG9j ayBiZWZvcmUgaXQgY2FuIGlzc3VlCj4gPiA+IGEgcmVzZXQuCj4gPiA+IAo+ID4gPiBUaGUgcHJv YmxlbSBpcyB0aGF0IHRoZSBkcml2ZXIncyB0aHJlYWQgbWF5IGFscmVhZHkgaG9sZCB0aGUgZGV2 aWNlCj4gPiA+IGxvY2suICBEdXJpbmcgYSBub3JtYWwgcHJvYmUgc2VxdWVuY2UsIGZvciBleGFt cGxlLCB0aGUgaW50ZXJmYWNlcyBnZXQKPiA+ID4gcHJvYmVkIGJ5IHRoZSBodWIgZHJpdmVyIHdo aWxlIGl0IG93bnMgdGhlIGRldmljZSBsb2NrLiAgQnV0IGZvciBwcm9iZXMKPiA+ID4gdW5kZXIg b3RoZXIgY2lyY3Vtc3RhbmNlcyAoZm9yIGV4YW1wbGUsIGlmIHRoZSB1c2VyIHdyaXRlcyB0byB0 aGUKPiA+ID4gZHJpdmVyJ3MgImJpbmQiIGF0dHJpYnV0ZSBpbiBzeXNmcyksIHRoZSBkZXZpY2Ug bG9jayBtaWdodCBub3QgYmUgaGVsZC4KPiA+ID4gCj4gPiA+IEEgZHJpdmVyIGNhbm5vdCB0ZWxs IHRoZXNlIHR3byBjYXNlcyBhcGFydC4gIFRoZSBvbmx5IHdheSB0byBtYWtlIGl0Cj4gPiA+IHdv cmsgYWxsIHRoZSB0aW1lIGlzIHRvIGhhdmUgdGhlIGNhbGxlciBfYWx3YXlzXyBob2xkIHRoZSBk ZXZpY2UgbG9jawo+ID4gPiB3aGlsZSB0aGUgZHJpdmVyIGlzIHByb2JlZCAob3IgdGhlIHJlbW92 ZWQsIGZvciB0aGF0IG1hdHRlcikuCj4gPiA+IAo+ID4gPiBBbGFuIFN0ZXJuCj4gPiAKPiA+IFRo YW5rcyBmb3IgdGhlIHJlcGx5IGFuZCBtb3JlIGRldGFpbCBhYm91dCB0aGUgYmFja2dyb3VkLiBJ J2QgbGlrZSB0bwo+ID4gaGF2ZSBhIGNvbmNsdXNpb24gYWJvdXQgaXQuIFBsZWFzZSBraW5kbHkg Y29ycmVjdCBtZSBpZiBteSB1bmRlcnN0YW5kaW5nCj4gPiBpcyB3cm9uZy4gUmVnYXJkaW5nIHRv IHRoZSAic3BlY2lhbC1wdXJwb3NlIGZsYWciLCBkbyB5b3UgbWVhbiB3ZSBjb3VsZAo+ID4gZmlu ZCBhIHBsYWNlIGluIFVTQiBzdWJzeXN0ZW0gdG8gaGF2ZSB0aGUgZmxhZyBzZXQgKG5vdCBzdXJl IGlmIGl0J3MKPiA+IGVhc3kgdG8gZmluZCBpdCkuIERyaXZlciBjb3JlIHdvdWxkIGJlIGJhc2Ug b24gdGhlIGZsYWcgdG8gZGVjaWRlIGlmIHdlCj4gPiBuZWVkIHRvIGhvbGQgdGhlIGRldmljZSdz IHBhcmVudCdzIGxvY2suCj4gCj4gWWVzLCBleGNlcHQgdGhhdCB0aGUgZmxhZyB3b3VsZCBub3Qg YmUgaW4gdGhlIFVTQiBzdWJzeXN0ZW0uICBJdCB3b3VsZCAKPiBiZSBpbiB0aGUgZGV2aWNlLCBk ZXZpY2VfdHlwZSwgb3IgYnVzX3R5cGUgc3RydWN0dXJlLCBzbyB0aGF0IHRoZSAKPiBkcml2ZXIg Y29yZSBjb3VsZCBhY2Nlc3MgaXQuCj4gCj4gQWxhbiBTdGVybgoKVGhhbmtzIGZvciB0aGUgcXVp Y2sgZmVlZGJhY2sgYW5kIHRoZSBzdWdnZXN0aW9uLiB3aWxsIHRyeSB0byBmaWd1cmUgb3V0IGhv dwppdCB3b3Jrcy4KCk1hcnRpbgotLS0KVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNl bmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXVzYiIgaW4KdGhlIGJvZHkgb2YgYSBtZXNz YWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAg aHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCg==