From: Alan Stern <stern@rowland.harvard.edu> To: martin_liu <liumartin@google.com> Cc: heikki.krogerus@linux.intel.com, <johan@kernel.org>, <gregkh@linuxfoundation.org>, <linux-kernel@vger.kernel.org>, <linux-usb@vger.kernel.org>, <jenhaochen@google.com> Subject: Re: [RFC PATCH v2] driver core: hold dev's parent lock when needed Date: Tue, 29 May 2018 10:07:07 -0400 (EDT) [thread overview] Message-ID: <Pine.LNX.4.44L0.1805291003470.1458-100000@iolanthe.rowland.org> (raw) In-Reply-To: <20180529070719.164626-1-liumartin@google.com> On Tue, 29 May 2018, martin_liu wrote: > SOC have internal I/O buses that can't be proved 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 it desribed 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. Thus, we introduce a lock flag > in device struct and driver core would lock the parent lock base > on the flag. For usbsystem, we set this flag when its device and > driver is matched and to keep original lock behavior in driver > core. > > Async probe could have more benefit after this patch. > > Signed-off-by: martin_liu <liumartin@google.com> > Suggested-by: Alan Stern <stern@rowland.harvard.edu> > --- > Changes in v2: > - take Alan's suggestion to introudce a flag to guide driver > core to hold device parent's lock. > > [v1]: https://lkml.org/lkml/2018/5/22/545 > > Currently, I have the flag set in USB subsystem match function. > Since I'm not familar with USB part, need some feedback to know > if they cover all the cases that original case driver core > protects. The match function is not the right place. Take Greg's suggestion and put the new flag in the bus_type structure instead. > drivers/base/bus.c | 16 ++++++++-------- > drivers/base/dd.c | 8 ++++---- > drivers/usb/common/ulpi.c | 16 ++++++++++++---- > drivers/usb/core/driver.c | 9 +++++++-- > drivers/usb/core/usb-acpi.c | 6 +++++- > drivers/usb/serial/bus.c | 4 +++- > include/linux/device.h | 1 + > 7 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/drivers/base/bus.c b/drivers/base/bus.c > index ef6183306b40..18ea94caec02 100644 > --- a/drivers/base/bus.c > +++ b/drivers/base/bus.c > @@ -184,10 +184,10 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, > > dev = bus_find_device_by_name(bus, NULL, buf); > if (dev && dev->driver == drv) { > - if (dev->parent) /* Needed for USB */ > + if (dev->parent && dev->need_parent_lock)/* Needed for USB */ The new need_parent_lock flag is self-explanatory. You can remove the "Needed for USB" comments here and elsewhere; instead just have a single comment where the flag is defined. Alan Stern
WARNING: multiple messages have this Message-ID (diff)
From: Alan Stern <stern@rowland.harvard.edu> To: martin_liu <liumartin@google.com> Cc: heikki.krogerus@linux.intel.com, johan@kernel.org, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, jenhaochen@google.com Subject: [RFC,v2] driver core: hold dev's parent lock when needed Date: Tue, 29 May 2018 10:07:07 -0400 (EDT) [thread overview] Message-ID: <Pine.LNX.4.44L0.1805291003470.1458-100000@iolanthe.rowland.org> (raw) On Tue, 29 May 2018, martin_liu wrote: > SOC have internal I/O buses that can't be proved 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 it desribed 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. Thus, we introduce a lock flag > in device struct and driver core would lock the parent lock base > on the flag. For usbsystem, we set this flag when its device and > driver is matched and to keep original lock behavior in driver > core. > > Async probe could have more benefit after this patch. > > Signed-off-by: martin_liu <liumartin@google.com> > Suggested-by: Alan Stern <stern@rowland.harvard.edu> > --- > Changes in v2: > - take Alan's suggestion to introudce a flag to guide driver > core to hold device parent's lock. > > [v1]: https://lkml.org/lkml/2018/5/22/545 > > Currently, I have the flag set in USB subsystem match function. > Since I'm not familar with USB part, need some feedback to know > if they cover all the cases that original case driver core > protects. The match function is not the right place. Take Greg's suggestion and put the new flag in the bus_type structure instead. > drivers/base/bus.c | 16 ++++++++-------- > drivers/base/dd.c | 8 ++++---- > drivers/usb/common/ulpi.c | 16 ++++++++++++---- > drivers/usb/core/driver.c | 9 +++++++-- > drivers/usb/core/usb-acpi.c | 6 +++++- > drivers/usb/serial/bus.c | 4 +++- > include/linux/device.h | 1 + > 7 files changed, 40 insertions(+), 20 deletions(-) > > diff --git a/drivers/base/bus.c b/drivers/base/bus.c > index ef6183306b40..18ea94caec02 100644 > --- a/drivers/base/bus.c > +++ b/drivers/base/bus.c > @@ -184,10 +184,10 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, > > dev = bus_find_device_by_name(bus, NULL, buf); > if (dev && dev->driver == drv) { > - if (dev->parent) /* Needed for USB */ > + if (dev->parent && dev->need_parent_lock)/* Needed for USB */ The new need_parent_lock flag is self-explanatory. You can remove the "Needed for USB" comments here and elsewhere; instead just have a single comment where the flag is defined. Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-05-29 14:07 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-22 14:12 [RFC] driver core: don't hold dev's parent lock when using async probe martin_liu 2018-05-22 14:12 ` martin_liu 2018-05-22 17:09 ` Alan Stern 2018-05-22 17:09 ` Alan Stern 2018-05-24 14:00 ` Martin Liu 2018-05-24 14:00 ` martin_liu 2018-05-24 15:02 ` Alan Stern 2018-05-24 15:02 ` Alan Stern 2018-05-24 16:05 ` Martin Liu 2018-05-24 16:05 ` martin_liu 2018-05-29 7:07 ` [RFC PATCH v2] driver core: hold dev's parent lock when needed martin_liu 2018-05-29 7:07 ` [RFC,v2] " martin_liu 2018-05-29 7:47 ` [RFC PATCH v2] " Greg KH 2018-05-29 7:47 ` [RFC,v2] " Greg Kroah-Hartman 2018-05-29 14:07 ` Alan Stern [this message] 2018-05-29 14:07 ` Alan Stern 2018-05-29 16:34 ` [RFC PATCH v3] " Martin Liu 2018-05-29 16:34 ` [RFC,v3] " martin_liu 2018-05-29 16:59 ` [RFC PATCH v3] " Greg KH 2018-05-29 16:59 ` [RFC,v3] " Greg Kroah-Hartman 2018-05-29 17:08 ` [RFC PATCH v3] " Andy Shevchenko 2018-05-29 17:08 ` [RFC,v3] " Andy Shevchenko 2018-05-29 18:49 ` [RFC PATCH v3] " Alan Stern 2018-05-29 18:49 ` [RFC,v3] " Alan Stern 2018-05-30 16:31 ` [PATCH v4] " Martin Liu 2018-05-30 16:31 ` [v4] " martin_liu 2018-05-30 17:21 ` [PATCH v4] " Alan Stern 2018-05-30 17:21 ` [v4] " Alan Stern 2018-05-31 6:31 ` [PATCH v4] " Greg KH 2018-05-31 6:31 ` [v4] " Greg Kroah-Hartman 2018-05-31 7:27 ` [PATCH v4] " Martin Liu 2018-05-31 7:27 ` [v4] " martin_liu 2018-05-31 18:55 ` [PATCH v4] " Andy Shevchenko 2018-05-31 18:55 ` [v4] " Andy Shevchenko 2018-05-29 15:28 ` [RFC PATCH v2] " Andy Shevchenko 2018-05-29 15:28 ` [RFC,v2] " Andy Shevchenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Pine.LNX.4.44L0.1805291003470.1458-100000@iolanthe.rowland.org \ --to=stern@rowland.harvard.edu \ --cc=gregkh@linuxfoundation.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=jenhaochen@google.com \ --cc=johan@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=liumartin@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.