From: Martin Liu <liumartin@google.com>
To: gregkh@linuxfoundation.org, heikki.krogerus@linux.intel.com,
johan@kernel.org, stern@rowland.harvard.edu,
andy.shevchenko@gmail.com
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
jenhaochen@google.com, Martin Liu <liumartin@google.com>
Subject: [PATCH v4] driver core: hold dev's parent lock when needed
Date: Thu, 31 May 2018 00:31:36 +0800 [thread overview]
Message-ID: <20180530163135.50081-1-liumartin@google.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1805291441110.1458-100000@iolanthe.rowland.org>
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 described in DT
(device tree).
In commit bf74ad5bc417 ("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 bus_type struct and driver core would lock the parent lock base
on the flag. For USB, we set this flag in usb_bus_type 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>
---
Changes in v4:
-fix comment and wording.
-follow the suggestion.
[v3]: https://lkml.org/lkml/2018/5/29/876
[v2]: https://lkml.org/lkml/2018/5/29/108
[v1]: https://lkml.org/lkml/2018/5/22/545
drivers/base/bus.c | 16 ++++++++--------
drivers/base/dd.c | 8 ++++----
drivers/usb/core/driver.c | 1 +
include/linux/device.h | 3 +++
4 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index ef6183306b40..8bfd27ec73d6 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->bus->need_parent_lock)
device_lock(dev->parent);
device_release_driver(dev);
- if (dev->parent)
+ if (dev->parent && dev->bus->need_parent_lock)
device_unlock(dev->parent);
err = count;
}
@@ -211,12 +211,12 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf,
dev = bus_find_device_by_name(bus, NULL, buf);
if (dev && dev->driver == NULL && driver_match_device(drv, dev)) {
- if (dev->parent) /* Needed for USB */
+ if (dev->parent && bus->need_parent_lock)
device_lock(dev->parent);
device_lock(dev);
err = driver_probe_device(drv, dev);
device_unlock(dev);
- if (dev->parent)
+ if (dev->parent && bus->need_parent_lock)
device_unlock(dev->parent);
if (err > 0) {
@@ -735,10 +735,10 @@ static int __must_check bus_rescan_devices_helper(struct device *dev,
int ret = 0;
if (!dev->driver) {
- if (dev->parent) /* Needed for USB */
+ if (dev->parent && dev->bus->need_parent_lock)
device_lock(dev->parent);
ret = device_attach(dev);
- if (dev->parent)
+ if (dev->parent && dev->bus->need_parent_lock)
device_unlock(dev->parent);
}
return ret < 0 ? ret : 0;
@@ -770,10 +770,10 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices);
int device_reprobe(struct device *dev)
{
if (dev->driver) {
- if (dev->parent) /* Needed for USB */
+ if (dev->parent && dev->bus->need_parent_lock)
device_lock(dev->parent);
device_release_driver(dev);
- if (dev->parent)
+ if (dev->parent && dev->bus->need_parent_lock)
device_unlock(dev->parent);
}
return bus_rescan_devices_helper(dev, NULL);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index c9f54089429b..7c09f73b96f3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -817,13 +817,13 @@ static int __driver_attach(struct device *dev, void *data)
return ret;
} /* ret > 0 means positive match */
- if (dev->parent) /* Needed for USB */
+ if (dev->parent && dev->bus->need_parent_lock)
device_lock(dev->parent);
device_lock(dev);
if (!dev->driver)
driver_probe_device(drv, dev);
device_unlock(dev);
- if (dev->parent)
+ if (dev->parent && dev->bus->need_parent_lock)
device_unlock(dev->parent);
return 0;
@@ -919,7 +919,7 @@ void device_release_driver_internal(struct device *dev,
struct device_driver *drv,
struct device *parent)
{
- if (parent)
+ if (parent && dev->bus->need_parent_lock)
device_lock(parent);
device_lock(dev);
@@ -927,7 +927,7 @@ void device_release_driver_internal(struct device *dev,
__device_release_driver(dev, parent);
device_unlock(dev);
- if (parent)
+ if (parent && dev->bus->need_parent_lock)
device_unlock(parent);
}
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 9792cedfc351..e76e95f62f76 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1922,4 +1922,5 @@ struct bus_type usb_bus_type = {
.name = "usb",
.match = usb_device_match,
.uevent = usb_uevent,
+ .need_parent_lock = true,
};
diff --git a/include/linux/device.h b/include/linux/device.h
index 477956990f5e..beca424395dd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -98,6 +98,8 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* @lock_key: Lock class key for use by the lock validator
* @force_dma: Assume devices on this bus should be set up by dma_configure()
* even if DMA capability is not explicitly described by firmware.
+ * @need_parent_lock: When probing or removing a device on this bus, the
+ * device core should lock the device's parent.
*
* A bus is a channel between the processor and one or more devices. For the
* purposes of the device model, all devices are connected via a bus, even if
@@ -138,6 +140,7 @@ struct bus_type {
struct lock_class_key lock_key;
bool force_dma;
+ bool need_parent_lock;
};
extern int __must_check bus_register(struct bus_type *bus);
--
2.17.0.921.gf22659ad46-goog
next prev parent reply other threads:[~2018-05-30 16:37 UTC|newest]
Thread overview: 18+ 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 17:09 ` Alan Stern
2018-05-24 14:00 ` Martin Liu
2018-05-24 15:02 ` Alan Stern
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:47 ` Greg KH
2018-05-29 14:07 ` Alan Stern
2018-05-29 16:34 ` [RFC PATCH v3] " Martin Liu
2018-05-29 16:59 ` Greg KH
2018-05-29 17:08 ` Andy Shevchenko
2018-05-29 18:49 ` Alan Stern
2018-05-30 16:31 ` Martin Liu [this message]
2018-05-30 17:21 ` [PATCH v4] " Alan Stern
2018-05-31 6:31 ` Greg KH
2018-05-31 7:27 ` Martin Liu
2018-05-31 18:55 ` Andy Shevchenko
2018-05-29 15:28 ` [RFC PATCH 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=20180530163135.50081-1-liumartin@google.com \
--to=liumartin@google.com \
--cc=andy.shevchenko@gmail.com \
--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=stern@rowland.harvard.edu \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®