From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751432AbeEVRJq (ORCPT ); Tue, 22 May 2018 13:09:46 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:51160 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751240AbeEVRJp (ORCPT ); Tue, 22 May 2018 13:09:45 -0400 Date: Tue, 22 May 2018 13:09:44 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: martin_liu cc: gregkh@linuxfoundation.org, , Subject: Re: [RFC] driver core: don't hold dev's parent lock when using async probe In-Reply-To: <20180522141227.GA118442@google.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 22 May 2018, martin_liu wrote: > SOC have internal I/O buses that can't be probed for devices. The > devices on the buses can be accessed directly without additinal > configuration required. This type of bus is represented as > "simple-bus". In some platforms, we name "soc" with "simple-bus" > attribute and many devices are hooked under and desribe them in DT > (device tree). > > In commit 'bf74ad5bc417 introduce ("[PATCH] Hold the device's > parent's lock during probe and remove")' to solve USB subsystem > lock sequence since usb device's characteristic. Thus "soc" > needs to be locked whenever a device and driver's probing > happen under "soc" bus. During this period, an async driver > tries to probe a device which is under the "soc" bus would be > blocked until previous driver finish the probing and release "soc" > lock. And the next probing under the "soc" bus need to wait for > async finish. Because of that, driver's async probe for init > time improvement will be shadowed. > > Since many devices don't have USB devices' characteristic, they > actually don't need parent's lock. However, in order to control > the risk and minimize the impact, we don't request parent's lock > only when a driver requests async probe. > > Async probe could have more benefit after we have this patch. > > Signed-off-by: martin_liu > --- > This RFC is asked to get some feedback since it involed driver > core and USB subsystem. I'm not familiar with USB subsystem and > 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 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: Alan Stern Message-Id: Date: Tue, 22 May 2018 13:09:44 -0400 (EDT) To: martin_liu Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org List-ID: T24gVHVlLCAyMiBNYXkgMjAxOCwgbWFydGluX2xpdSB3cm90ZToKCj4gU09DIGhhdmUgaW50ZXJu YWwgSS9PIGJ1c2VzIHRoYXQgY2FuJ3QgYmUgcHJvYmVkIGZvciBkZXZpY2VzLiBUaGUKPiBkZXZp Y2VzIG9uIHRoZSBidXNlcyBjYW4gYmUgYWNjZXNzZWQgZGlyZWN0bHkgd2l0aG91dCBhZGRpdGlu YWwKPiBjb25maWd1cmF0aW9uIHJlcXVpcmVkLiBUaGlzIHR5cGUgb2YgYnVzIGlzIHJlcHJlc2Vu dGVkIGFzCj4gInNpbXBsZS1idXMiLiBJbiBzb21lIHBsYXRmb3Jtcywgd2UgbmFtZSAic29jIiB3 aXRoICJzaW1wbGUtYnVzIgo+IGF0dHJpYnV0ZSBhbmQgbWFueSBkZXZpY2VzIGFyZSBob29rZWQg dW5kZXIgYW5kIGRlc3JpYmUgdGhlbSBpbiBEVAo+IChkZXZpY2UgdHJlZSkuCj4gCj4gSW4gY29t bWl0ICdiZjc0YWQ1YmM0MTcgaW50cm9kdWNlICgiW1BBVENIXSBIb2xkIHRoZSBkZXZpY2Uncwo+ IHBhcmVudCdzIGxvY2sgZHVyaW5nIHByb2JlIGFuZCByZW1vdmUiKScgdG8gc29sdmUgVVNCIHN1 YnN5c3RlbQo+IGxvY2sgc2VxdWVuY2Ugc2luY2UgdXNiIGRldmljZSdzIGNoYXJhY3RlcmlzdGlj LiBUaHVzICJzb2MiCj4gbmVlZHMgdG8gYmUgbG9ja2VkIHdoZW5ldmVyIGEgZGV2aWNlIGFuZCBk cml2ZXIncyBwcm9iaW5nCj4gaGFwcGVuIHVuZGVyICJzb2MiIGJ1cy4gRHVyaW5nIHRoaXMgcGVy aW9kLCBhbiBhc3luYyBkcml2ZXIKPiB0cmllcyB0byBwcm9iZSBhIGRldmljZSB3aGljaCBpcyB1 bmRlciB0aGUgInNvYyIgYnVzIHdvdWxkIGJlCj4gYmxvY2tlZCB1bnRpbCBwcmV2aW91cyBkcml2 ZXIgZmluaXNoIHRoZSBwcm9iaW5nIGFuZCByZWxlYXNlICJzb2MiCj4gbG9jay4gQW5kIHRoZSBu ZXh0IHByb2JpbmcgdW5kZXIgdGhlICJzb2MiIGJ1cyBuZWVkIHRvIHdhaXQgZm9yCj4gYXN5bmMg ZmluaXNoLiBCZWNhdXNlIG9mIHRoYXQsIGRyaXZlcidzIGFzeW5jIHByb2JlIGZvciBpbml0Cj4g dGltZSBpbXByb3ZlbWVudCB3aWxsIGJlIHNoYWRvd2VkLgo+IAo+IFNpbmNlIG1hbnkgZGV2aWNl cyBkb24ndCBoYXZlIFVTQiBkZXZpY2VzJyBjaGFyYWN0ZXJpc3RpYywgdGhleQo+IGFjdHVhbGx5 IGRvbid0IG5lZWQgcGFyZW50J3MgbG9jay4gSG93ZXZlciwgaW4gb3JkZXIgdG8gY29udHJvbAo+ IHRoZSByaXNrIGFuZCBtaW5pbWl6ZSB0aGUgaW1wYWN0LCB3ZSBkb24ndCByZXF1ZXN0IHBhcmVu dCdzIGxvY2sKPiBvbmx5IHdoZW4gYSBkcml2ZXIgcmVxdWVzdHMgYXN5bmMgcHJvYmUuCj4gCj4g QXN5bmMgcHJvYmUgY291bGQgaGF2ZSBtb3JlIGJlbmVmaXQgYWZ0ZXIgd2UgaGF2ZSB0aGlzIHBh dGNoLgo+IAo+IFNpZ25lZC1vZmYtYnk6IG1hcnRpbl9saXUgPGxpdW1hcnRpbkBnb29nbGUuY29t Pgo+IC0tLQo+IFRoaXMgUkZDIGlzIGFza2VkIHRvIGdldCBzb21lIGZlZWRiYWNrIHNpbmNlIGl0 IGludm9sZWQgZHJpdmVyCj4gY29yZSBhbmQgVVNCIHN1YnN5c3RlbS4gSSdtIG5vdCBmYW1pbGlh ciB3aXRoIFVTQiBzdWJzeXN0ZW0gYW5kCj4gbm90IHN1cmUgaWYgd2Ugc3RpbGwgbmVlZCAnYmY3 NGFkNWJjNDE3ICgiW1BBVENIXSBIb2xkIHRoZQo+IGRldmljZSdzIHBhcmVudCdzIGxvY2sgZHVy aW5nIHByb2JlIGFuZCByZW1vdmUiKScgc2luY2UgaXQgaGFzCj4gYmVlbiB0aGVyZSBvdmVyIDEw IHllYXJzLiBJZiB3ZSBzdGlsbCBuZWVkIGl0IGFuZCBoYXJkIHRvIGZpeCBpdAo+ICwgdGhlIHNp bXBsZSB3YXkgaXMgdG8gZmluZCBhIHBsYWNlIG5vdCB0byBhbGxvdyBVU0Igc3Vic3lzdGVtCj4g ZHJpdmVycyB0byBoYXZlIGFzeW5jIHByb2JlIGNhcGFiaWxpdHkuIEFueSBzdWdnZXN0aW9uIGlz IHdlbGNvbWUuCgpJIGRvbid0IHRoaW5rIHRoZSAiYWxsb3dzX2FzeW5jX3Byb2JpbmciIGF0dHJp YnV0ZSBpcyB0aGUgYmVzdCB3YXkgdG8gCmF0dGFjayB0aGlzLiAgU29tZSBvdGhlciBhcHByb2Fj aCwgbGlrZSBhIHNwZWNpYWwtcHVycG9zZSBmbGFnLCBtaWdodCAKYmUgYmV0dGVyLgoKWWVzLCBV U0Igc3RpbGwgbmVlZHMgdG8gaGF2ZSBwYXJlbnQncyBsb2NrcyBoZWxkIGR1cmluZyBwcm9iaW5n LiAgCkhlcmUncyB0aGUgcmVhc29uLiAgQSBVU0IgZGV2aWNlIGNhbiBoYXZlIG11bHRpcGxlIGlu dGVyZmFjZXMsIGVhY2gKYm91bmQgdG8gaXRzIG93biBkcml2ZXIuICBBIGRyaXZlciBtYXkgc29t ZXRpbWVzIG5lZWQgdG8gaXNzdWUgYSByZXNldCwKYnV0IGluIFVTQiB0aGVyZSdzIG5vIHdheSB0 byByZXNldCBhIHNpbmdsZSBpbnRlcmZhY2UuICBPbmx5IHRoZSBlbnRpcmUKZGV2aWNlIGNhbiBi ZSByZXNldCwgYW5kIG9mIGNvdXJzZSB0aGlzIGFmZmVjdHMgYWxsIHRoZSBpbnRlcmZhY2VzLiAg ClRoZXJlZm9yZSBhIGRyaXZlciBuZWVkcyB0byBhY3F1aXJlIHRoZSBkZXZpY2UgbG9jayBiZWZv cmUgaXQgY2FuIGlzc3VlCmEgcmVzZXQuCgpUaGUgcHJvYmxlbSBpcyB0aGF0IHRoZSBkcml2ZXIn cyB0aHJlYWQgbWF5IGFscmVhZHkgaG9sZCB0aGUgZGV2aWNlCmxvY2suICBEdXJpbmcgYSBub3Jt YWwgcHJvYmUgc2VxdWVuY2UsIGZvciBleGFtcGxlLCB0aGUgaW50ZXJmYWNlcyBnZXQKcHJvYmVk IGJ5IHRoZSBodWIgZHJpdmVyIHdoaWxlIGl0IG93bnMgdGhlIGRldmljZSBsb2NrLiAgQnV0IGZv ciBwcm9iZXMKdW5kZXIgb3RoZXIgY2lyY3Vtc3RhbmNlcyAoZm9yIGV4YW1wbGUsIGlmIHRoZSB1 c2VyIHdyaXRlcyB0byB0aGUKZHJpdmVyJ3MgImJpbmQiIGF0dHJpYnV0ZSBpbiBzeXNmcyksIHRo ZSBkZXZpY2UgbG9jayBtaWdodCBub3QgYmUgaGVsZC4KCkEgZHJpdmVyIGNhbm5vdCB0ZWxsIHRo ZXNlIHR3byBjYXNlcyBhcGFydC4gIFRoZSBvbmx5IHdheSB0byBtYWtlIGl0CndvcmsgYWxsIHRo ZSB0aW1lIGlzIHRvIGhhdmUgdGhlIGNhbGxlciBfYWx3YXlzXyBob2xkIHRoZSBkZXZpY2UgbG9j awp3aGlsZSB0aGUgZHJpdmVyIGlzIHByb2JlZCAob3IgdGhlIHJlbW92ZWQsIGZvciB0aGF0IG1h dHRlcikuCgpBbGFuIFN0ZXJuCi0tLQpUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2Vu ZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtdXNiIiBpbgp0aGUgYm9keSBvZiBhIG1lc3Nh Z2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZwpNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBo dHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwK