From: Stuart Hayes <stuart.w.hayes@gmail.com>
To: linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Martin Belanger <Martin.Belanger@dell.com>,
Oliver O'Halloran <oohall@gmail.com>,
Daniel Wagner <dwagner@suse.de>, Keith Busch <kbusch@kernel.org>,
Lukas Wunner <lukas@wunner.de>,
David Jeffery <djeffery@redhat.com>,
Jeremy Allison <jallison@ciq.com>, Jens Axboe <axboe@fb.com>,
Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
linux-nvme@lists.infradead.org,
Nathan Chancellor <nathan@kernel.org>,
Jan Kiszka <jan.kiszka@seimens.com>,
Bert Karwatzki <spasswolf@web.de>
Cc: Stuart Hayes <stuart.w.hayes@gmail.com>
Subject: [PATCH v9 3/4] driver core: shut down devices asynchronously
Date: Wed, 9 Oct 2024 12:57:45 -0500 [thread overview]
Message-ID: <20241009175746.46758-4-stuart.w.hayes@gmail.com> (raw)
In-Reply-To: <20241009175746.46758-1-stuart.w.hayes@gmail.com>
Add code to allow asynchronous shutdown of devices, ensuring that each
device is shut down before its parents & suppliers. Any devices that are
ordered in the devices_kset list such that a parent or supplier would
shut down before its child or consumer are shut down synchronously (as
they are without this patch).
Only devices with drivers that have async_shutdown_enable enabled will be
shut down asynchronously.
This can dramatically reduce system shutdown/reboot time on systems that
have multiple devices that take many seconds to shut down (like certain
NVMe drives). On one system tested, the shutdown time went from 11 minutes
without this patch to 55 seconds with the patch.
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
Signed-off-by: David Jeffery <djeffery@redhat.com>
---
drivers/base/base.h | 4 ++
drivers/base/core.c | 81 ++++++++++++++++++++++++++++++++++-
include/linux/device/driver.h | 2 +
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 8cf04a557bdb..ea18aa70f151 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -10,6 +10,7 @@
* shared outside of the drivers/base/ directory.
*
*/
+#include <linux/async.h>
#include <linux/notifier.h>
/**
@@ -97,6 +98,8 @@ struct driver_private {
* the device; typically because it depends on another driver getting
* probed first.
* @async_driver - pointer to device driver awaiting probe via async_probe
+ * @shutdown_after - used during device shutdown to ensure correct shutdown
+ * ordering.
* @device - pointer back to the struct device that this structure is
* associated with.
* @dead - This device is currently either in the process of or has been
@@ -114,6 +117,7 @@ struct device_private {
struct list_head deferred_probe;
const struct device_driver *async_driver;
char *deferred_probe_reason;
+ async_cookie_t shutdown_after;
struct device *device;
u8 dead:1;
};
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 4482382fb947..bde3fdeafbdb 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -9,6 +9,7 @@
*/
#include <linux/acpi.h>
+#include <linux/async.h>
#include <linux/blkdev.h>
#include <linux/cleanup.h>
#include <linux/cpufreq.h>
@@ -3524,6 +3525,7 @@ static int device_private_init(struct device *dev)
klist_init(&dev->p->klist_children, klist_children_get,
klist_children_put);
INIT_LIST_HEAD(&dev->p->deferred_probe);
+ dev->p->shutdown_after = 0;
return 0;
}
@@ -4779,6 +4781,8 @@ int device_change_owner(struct device *dev, kuid_t kuid, kgid_t kgid)
}
EXPORT_SYMBOL_GPL(device_change_owner);
+static ASYNC_DOMAIN(sd_domain);
+
static void shutdown_one_device(struct device *dev)
{
/* hold lock to avoid race with probe/release */
@@ -4814,12 +4818,61 @@ static void shutdown_one_device(struct device *dev)
put_device(dev->parent);
}
+static bool device_wants_async_shutdown(struct device *dev)
+{
+ if (dev->driver && dev->driver->async_shutdown_enable)
+ return true;
+
+ return false;
+}
+
+/**
+ * shutdown_one_device_async
+ * @data: the pointer to the struct device to be shutdown
+ * @cookie: not used
+ *
+ * Shuts down one device, after waiting for previous device to shut down (for
+ * synchronous shutdown) or waiting for device's last child or consumer to
+ * be shutdown (for async shutdown).
+ *
+ * shutdown_after is set to the shutdown cookie of the last child or consumer
+ * of this device (if any).
+ */
+static void shutdown_one_device_async(void *data, async_cookie_t cookie)
+{
+ struct device *dev = data;
+ async_cookie_t wait = cookie;
+
+ if (device_wants_async_shutdown(dev)) {
+ wait = dev->p->shutdown_after + 1;
+ /*
+ * To prevent system hang, revert to sync shutdown in the event
+ * that shutdown_after would make this shutdown wait for a
+ * shutdown that hasn't been scheduled yet.
+ *
+ * This can happen if a parent or supplier is not ordered in the
+ * devices_kset list before a child or consumer, which is not
+ * expected.
+ */
+ if (wait > cookie) {
+ wait = cookie;
+ dev_warn(dev, "Unsafe shutdown ordering, forcing sync order\n");
+ }
+ }
+
+ async_synchronize_cookie_domain(wait, &sd_domain);
+ shutdown_one_device(dev);
+}
+
/**
* device_shutdown - call ->shutdown() on each device to shutdown.
*/
void device_shutdown(void)
{
struct device *dev, *parent;
+ async_cookie_t cookie;
+ struct device_link *link;
+ int idx;
wait_for_device_probe();
device_block_probing();
@@ -4850,11 +4903,37 @@ void device_shutdown(void)
list_del_init(&dev->kobj.entry);
spin_unlock(&devices_kset->list_lock);
- shutdown_one_device(dev);
+ get_device(dev);
+ get_device(parent);
+
+ cookie = async_schedule_domain(shutdown_one_device_async,
+ dev, &sd_domain);
+ /*
+ * Ensure any parent & suppliers will wait for this device to
+ * shut down
+ */
+ if (parent) {
+ parent->p->shutdown_after = cookie;
+ put_device(parent);
+ }
+
+ idx = device_links_read_lock();
+ list_for_each_entry_rcu(link, &dev->links.suppliers, c_node,
+ device_links_read_lock_held()) {
+ /*
+ * sync_state_only devlink consumers aren't dependent on
+ * suppliers
+ */
+ if (!device_link_flag_is_sync_state_only(link->flags))
+ link->supplier->p->shutdown_after = cookie;
+ }
+ device_links_read_unlock(idx);
+ put_device(dev);
spin_lock(&devices_kset->list_lock);
}
spin_unlock(&devices_kset->list_lock);
+ async_synchronize_full_domain(&sd_domain);
}
/*
diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
index 5c04b8e3833b..14c9211b82d6 100644
--- a/include/linux/device/driver.h
+++ b/include/linux/device/driver.h
@@ -56,6 +56,7 @@ enum probe_type {
* @mod_name: Used for built-in modules.
* @suppress_bind_attrs: Disables bind/unbind via sysfs.
* @probe_type: Type of the probe (synchronous or asynchronous) to use.
+ * @async_shutdown_enable: Enables devices to be shutdown asynchronously.
* @of_match_table: The open firmware table.
* @acpi_match_table: The ACPI match table.
* @probe: Called to query the existence of a specific device,
@@ -102,6 +103,7 @@ struct device_driver {
bool suppress_bind_attrs; /* disables bind/unbind via sysfs */
enum probe_type probe_type;
+ bool async_shutdown_enable;
const struct of_device_id *of_match_table;
const struct acpi_device_id *acpi_match_table;
--
2.39.3
next prev parent reply other threads:[~2024-10-09 17:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-09 17:57 [PATCH v9 0/4] shut down devices asynchronously Stuart Hayes
2024-10-09 17:57 ` [PATCH v9 1/4] driver core: don't always lock parent in shutdown Stuart Hayes
2024-10-09 17:57 ` [PATCH v9 2/4] driver core: separate function to shutdown one device Stuart Hayes
2024-10-09 17:57 ` Stuart Hayes [this message]
2024-10-09 17:57 ` [PATCH v9 4/4] nvme-pci: Make driver prefer asynchronous shutdown Stuart Hayes
2024-10-11 4:22 ` [PATCH v9 0/4] shut down devices asynchronously Michael Kelley
2024-10-11 15:52 ` Laurence Oberman
2024-10-18 3:26 ` Michael Kelley
2024-10-18 5:49 ` Greg Kroah-Hartman
[not found] ` <ZxInC1U7WiB7FNkJ@wunner.de>
2024-10-18 9:37 ` Greg Kroah-Hartman
2024-10-19 0:27 ` stuart hayes
2024-10-20 0:24 ` Michael Kelley
2024-10-29 15:32 ` Christoph Hellwig
2025-01-30 3:06 ` [PATCH] driver core: optimize async device shutdown Sultan Alsawaf
2025-01-30 17:12 ` [PATCH v2] " Sultan Alsawaf
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=20241009175746.46758-4-stuart.w.hayes@gmail.com \
--to=stuart.w.hayes@gmail.com \
--cc=Martin.Belanger@dell.com \
--cc=axboe@fb.com \
--cc=djeffery@redhat.com \
--cc=dwagner@suse.de \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=jallison@ciq.com \
--cc=jan.kiszka@seimens.com \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=lukas@wunner.de \
--cc=nathan@kernel.org \
--cc=oohall@gmail.com \
--cc=rafael@kernel.org \
--cc=sagi@grimberg.me \
--cc=spasswolf@web.de \
/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