lkml.org 
[lkml]   [2018]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v2] driver core: hold dev's parent lock when needed
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

\
 
 \ /
  Last update: 2018-05-29 16:07    [W:0.127 / U:10.660 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site