[PATCH 1/3] driver core: Support asynchronous driver shutdown

[PATCH 1/3] driver core: Support asynchronous driver shutdown

[Tanjore Suresh]

This changes the bus driver interface to take in a flag to indicate
whether a bus and associated devices are willing to participate in
the asynchronous shutdown. If this flag is not set bus driver
implementation will follow synchronous shutdown semantics.

Signed-off-by: Tanjore Suresh <tansuresh@google.com>
---
 drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
 include/linux/device/bus.h | 10 ++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3d6430eb0c6a..359e7067e8b8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4479,6 +4479,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
 void device_shutdown(void)
 {
 	struct device *dev, *parent;
+	LIST_HEAD(async_shutdown_list);
 
 	wait_for_device_probe();
 	device_block_probing();
@@ -4523,7 +4524,14 @@ void device_shutdown(void)
 				dev_info(dev, "shutdown_pre\n");
 			dev->class->shutdown_pre(dev);
 		}
-		if (dev->bus && dev->bus->shutdown) {
+
+		if (dev->bus && dev->bus->shutdown_pre) {
+			if (initcall_debug)
+				dev_info(dev, "shutdown_pre\n");
+			dev->bus->shutdown_pre(dev);
+			list_add(&dev->kobj.entry,
+				&async_shutdown_list);
+		} else if (dev->bus && dev->bus->shutdown) {
 			if (initcall_debug)
 				dev_info(dev, "shutdown\n");
 			dev->bus->shutdown(dev);
@@ -4543,6 +4551,35 @@ void device_shutdown(void)
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+
+	/*
+	 * Second pass spin for only devices, that have configured
+	 * Asynchronous shutdown.
+	 */
+	while (!list_empty(&async_shutdown_list)) {
+		dev = list_entry(async_shutdown_list.next, struct device,
+				kobj.entry);
+		parent = get_device(dev->parent);
+		get_device(dev);
+		/*
+		 * Make sure the device is off the  list
+		 */
+		list_del_init(&dev->kobj.entry);
+		if (parent)
+			device_lock(parent);
+		device_lock(dev);
+		if (dev->bus && dev->bus->shutdown_post) {
+			if (initcall_debug)
+				dev_info(dev,
+				"shutdown_post called\n");
+			dev->bus->shutdown_post(dev);
+		}
+		device_unlock(dev);
+		if (parent)
+			device_unlock(parent);
+		put_device(dev);
+		put_device(parent);
+	}
 }
 
 /*
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index a039ab809753..e261819601e9 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -49,6 +49,14 @@ struct fwnode_handle;
  *		will never get called until they do.
  * @remove:	Called when a device removed from this bus.
  * @shutdown:	Called at shut-down time to quiesce the device.
+ * @shutdown_pre:	Called at the shutdown-time to start the shutdown
+ *			process on the device. This entry point will be called
+ *			only when the bus driver has indicated it would like
+ *			to participate in asynchronous shutdown completion.
+ * @shutdown_post:	Called at shutdown-time  to complete the shutdown
+ *			process of the device. This entry point will be called
+ *			only when the bus drive has indicated it would like to
+ *			participate in the asynchronous shutdown completion.
  *
  * @online:	Called to put the device back online (after offlining it).
  * @offline:	Called to put the device offline for hot-removal. May fail.
@@ -93,6 +101,8 @@ struct bus_type {
 	void (*sync_state)(struct device *dev);
 	void (*remove)(struct device *dev);
 	void (*shutdown)(struct device *dev);
+	void (*shutdown_pre)(struct device *dev);
+	void (*shutdown_post)(struct device *dev);
 
 	int (*online)(struct device *dev);
 	int (*offline)(struct device *dev);
-- 
2.35.1.1021.g381101b075-goog

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Greg Kroah-Hartman]

On Thu, Mar 24, 2022 at 02:34:45PM -0700, Tanjore Suresh wrote:
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -49,6 +49,14 @@ struct fwnode_handle;
>   *		will never get called until they do.
>   * @remove:	Called when a device removed from this bus.
>   * @shutdown:	Called at shut-down time to quiesce the device.
> + * @shutdown_pre:	Called at the shutdown-time to start the shutdown
> + *			process on the device. This entry point will be called
> + *			only when the bus driver has indicated it would like
> + *			to participate in asynchronous shutdown completion.
> + * @shutdown_post:	Called at shutdown-time  to complete the shutdown
> + *			process of the device. This entry point will be called
> + *			only when the bus drive has indicated it would like to
> + *			participate in the asynchronous shutdown completion.

Sorry, but no, this should not be needed expecially as you did not offer
any justification or reason to do so.

Nor did you send the remaining changes in the series to me, and why
would these be "trivial"?

Please work with others at Google who know how to submit changes to the
kernel first and get their review and signed-off-by on the changes
before sending them out again.

good luck!

greg k-h

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Bjorn Helgaas]

[dropped "trivial" from cc]

On Thu, Mar 24, 2022 at 02:34:45PM -0700, Tanjore Suresh wrote:
> This changes the bus driver interface to take in a flag to indicate
> whether a bus and associated devices are willing to participate in
> the asynchronous shutdown. If this flag is not set bus driver
> implementation will follow synchronous shutdown semantics.
> 
> Signed-off-by: Tanjore Suresh <tansuresh@google.com>

There's useful functionality here.  Some hints to make it more
digestable:

- Add a cover letter to give an overview.  The patches themselves
  should be sent as responses to the cover letter so everything is
  connected in the email archives.

  [1] is a nice example of what this looks like.  You can currently
  find your series as [2], which searches for everything from you, but
  there's no single permanent URL that finds the whole series.

- Send the whole series (cover letter + patches) to everybody, so
  people can see the context and where each piece fits.

  No need to CC the "trivial@kernel.org" list.  That's for things like
  tiny, obviously correct patches that can be reviewed very quickly.

- Wait a week or so for any comments on this series before sending a
  revised v2.  When you send a v2, use "git format-patch -v 2" or
  similar to mark it as v2.  Also include notes what what changed
  between v1 (this posting) and v2.  [1] has nice examples of how to
  do that, both in the cover letter and the individual patches.

- Update this commit log so it matches the code (there is no longer a
  flag).

- Write commit logs in imperative mood; see [3, 4].

  In this case, your commit log should probably have two parts: the
  first to outline the problem, and the second to say what this
  patches does about it, e.g., something like this:

    Driver .shutdown() methods are all run serially, so there's no
    parallelism even across unrelated devices.

    Add an optional asynchronous shutdown method so drivers can
    schedule work to be done in parallel.

A few code comments below.

[1] https://lore.kernel.org/linux-pci/20220325093827.4983-1-pali@kernel.org/T/#t
[2] https://lore.kernel.org/all/?q=f%3Atansuresh
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-tip.rst?id=v5.16#n134
[4] https://chris.beams.io/posts/git-commit/

> ---
>  drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
>  include/linux/device/bus.h | 10 ++++++++++
>  2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 3d6430eb0c6a..359e7067e8b8 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4479,6 +4479,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
>  void device_shutdown(void)
>  {
>  	struct device *dev, *parent;
> +	LIST_HEAD(async_shutdown_list);
>  
>  	wait_for_device_probe();
>  	device_block_probing();
> @@ -4523,7 +4524,14 @@ void device_shutdown(void)
>  				dev_info(dev, "shutdown_pre\n");
>  			dev->class->shutdown_pre(dev);
>  		}
> -		if (dev->bus && dev->bus->shutdown) {
> +
> +		if (dev->bus && dev->bus->shutdown_pre) {
> +			if (initcall_debug)
> +				dev_info(dev, "shutdown_pre\n");
> +			dev->bus->shutdown_pre(dev);
> +			list_add(&dev->kobj.entry,
> +				&async_shutdown_list);
> +		} else if (dev->bus && dev->bus->shutdown) {
>  			if (initcall_debug)
>  				dev_info(dev, "shutdown\n");
>  			dev->bus->shutdown(dev);
> @@ -4543,6 +4551,35 @@ void device_shutdown(void)
>  		spin_lock(&devices_kset->list_lock);
>  	}
>  	spin_unlock(&devices_kset->list_lock);
> +
> +	/*
> +	 * Second pass spin for only devices, that have configured
> +	 * Asynchronous shutdown.
> +	 */
> +	while (!list_empty(&async_shutdown_list)) {
> +		dev = list_entry(async_shutdown_list.next, struct device,
> +				kobj.entry);
> +		parent = get_device(dev->parent);
> +		get_device(dev);
> +		/*
> +		 * Make sure the device is off the  list
> +		 */
> +		list_del_init(&dev->kobj.entry);
> +		if (parent)
> +			device_lock(parent);
> +		device_lock(dev);
> +		if (dev->bus && dev->bus->shutdown_post) {
> +			if (initcall_debug)
> +				dev_info(dev,
> +				"shutdown_post called\n");
> +			dev->bus->shutdown_post(dev);
> +		}
> +		device_unlock(dev);
> +		if (parent)
> +			device_unlock(parent);
> +		put_device(dev);
> +		put_device(parent);
> +	}

I'm not a driver core expert, but AFAICS, the existing model is that
.shutdown() is always synchronous.  We call it for each device
serially.

And your proposal is to allow some shutdown processing to happen in
parallel, by adding .shutdown_pre() to *schedule* work that can happen
after .shutdown_pre() returns, and .shutdown_post() to *wait* for all
that scheduled work to complete.

IIUC, .shutdown_post() is completely synchronous, just like the
existing .shutdown() is, so it seems unnecessary to add it.

Seems like it would be simpler to add an optional .shutdown_async()
method.  This method would be called in a loop *before* the existing
loop that calls .shutdown(), and it could start the async work.

Drivers that implement .shutdown_async() would at the same time update
their .shutdown() methods to wait for all the work started by
.shutdown_async().

>  }
>  
>  /*
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index a039ab809753..e261819601e9 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -49,6 +49,14 @@ struct fwnode_handle;
>   *		will never get called until they do.
>   * @remove:	Called when a device removed from this bus.
>   * @shutdown:	Called at shut-down time to quiesce the device.
> + * @shutdown_pre:	Called at the shutdown-time to start the shutdown
> + *			process on the device. This entry point will be called
> + *			only when the bus driver has indicated it would like
> + *			to participate in asynchronous shutdown completion.
> + * @shutdown_post:	Called at shutdown-time  to complete the shutdown
> + *			process of the device. This entry point will be called
> + *			only when the bus drive has indicated it would like to
> + *			participate in the asynchronous shutdown completion.
>   *
>   * @online:	Called to put the device back online (after offlining it).
>   * @offline:	Called to put the device offline for hot-removal. May fail.
> @@ -93,6 +101,8 @@ struct bus_type {
>  	void (*sync_state)(struct device *dev);
>  	void (*remove)(struct device *dev);
>  	void (*shutdown)(struct device *dev);
> +	void (*shutdown_pre)(struct device *dev);
> +	void (*shutdown_post)(struct device *dev);
>  
>  	int (*online)(struct device *dev);
>  	int (*offline)(struct device *dev);
> -- 
> 2.35.1.1021.g381101b075-goog
> 

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Sagi Grimberg]

On 12/19/23 15:56, Christoph Hellwig wrote:
> On Tue, Dec 19, 2023 at 03:49:18PM +0200, Sagi Grimberg wrote:
>>>> I could change this so that the existing shutdown() method
>>>> is always called, and the device is only added to the async_shutdown_list
>>>> if a shutdown_post() (or as requested below, shutdown_wait())
>>>> method is defined for the device.
>>>
>>> Yes, that's what I mean.
>>
>> I think its usually better to separate sync vs async interfaces. However
>> I assume that the suggested interface exists elsewhere in the kernel, so
>> its not a big deal.
> 
> I don't think we have async shutdown anywhere else.

I meant the case where an interface is either sync where it needs to
complete inside the callback, or it can be async because it is also
paired with an optional _wait|_post|_end handler.

> It's also not really an async interface is the classic sense, but more a
> fire now and then wait for completion later interface, i.e. no notication
> on completion but pure polling.

It could have been waiting on a notify/completion for that matter.

I find the above a bit ambiguous, but maybe its just me, so I don't
care much about it. Your suggestion is fine too.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Christoph Hellwig]

On Tue, Dec 19, 2023 at 03:49:18PM +0200, Sagi Grimberg wrote:
>>> I could change this so that the existing shutdown() method
>>> is always called, and the device is only added to the async_shutdown_list
>>> if a shutdown_post() (or as requested below, shutdown_wait())
>>> method is defined for the device.
>>
>> Yes, that's what I mean.
>
> I think its usually better to separate sync vs async interfaces. However
> I assume that the suggested interface exists elsewhere in the kernel, so
> its not a big deal.

I don't think we have async shutdown anywhere else.

It's also not really an async interface is the classic sense, but more a
fire now and then wait for completion later interface, i.e. no notication
on completion but pure polling.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Sagi Grimberg]

On 12/19/23 08:21, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 10:19:43PM -0800, Jeremy Allison wrote:
>>> Is there any reason to have a separate shutdown_pre method?
>>> Especially with all the method wrapping in the driver core, yet
>>> another method just keeps confusing everyone.
>>
>> Currently in the patch the existence of a shutdown_pre() method
>> for a device causes it to be added to the async_shutdown_list
>> which is walked to reap the completion status after all the
>> calls to shutdown_pre().
>>
>> I could change this so that the existing shutdown() method
>> is always called, and the device is only added to the async_shutdown_list
>> if a shutdown_post() (or as requested below, shutdown_wait())
>> method is defined for the device.
> 
> Yes, that's what I mean.

I think its usually better to separate sync vs async interfaces. However
I assume that the suggested interface exists elsewhere in the kernel, so
its not a big deal.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Christoph Hellwig]

On Mon, Dec 18, 2023 at 10:19:43PM -0800, Jeremy Allison wrote:
>> Is there any reason to have a separate shutdown_pre method?
>> Especially with all the method wrapping in the driver core, yet
>> another method just keeps confusing everyone.
>
> Currently in the patch the existence of a shutdown_pre() method
> for a device causes it to be added to the async_shutdown_list
> which is walked to reap the completion status after all the
> calls to shutdown_pre().
>
> I could change this so that the existing shutdown() method
> is always called, and the device is only added to the async_shutdown_list
> if a shutdown_post() (or as requested below, shutdown_wait())
> method is defined for the device.

Yes, that's what I mean.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Jeremy Allison]

On Tue, Dec 19, 2023 at 06:33:18AM +0100, Christoph Hellwig wrote:
>On Thu, Dec 14, 2023 at 04:03:56PM -0800, Jeremy Allison wrote:
>> From: Tanjore Suresh <tansuresh@google.com>
>>
>> This changes the bus driver interface with additional entry points
>> to enable devices to implement asynchronous shutdown. The existing
>> synchronous interface to shutdown is unmodified and retained for
>> backward compatibility. shutdown_pre is only called if a matching
>> shutdown_post function is also registered, otherwise the synchronous
>> synchronous interface is used.
>>
>> This changes the common device shutdown code to enable devices to
>> participate in asynchronous shutdown implementation.
>
>Is there any reason to have a separate shutdown_pre method?
>Especially with all the method wrapping in the driver core, yet
>another method just keeps confusing everyone.

Currently in the patch the existence of a shutdown_pre() method
for a device causes it to be added to the async_shutdown_list
which is walked to reap the completion status after all the
calls to shutdown_pre().

I could change this so that the existing shutdown() method
is always called, and the device is only added to the async_shutdown_list
if a shutdown_post() (or as requested below, shutdown_wait())
method is defined for the device.

That makes sense to me and I'm happy to make that change.

>And on the post side, might shutdown_wait be a better name to
>describe the operation, but I'm open to opinions.

I'm happy to change shutdown_post() -> shudown_wait().

shutdown_pre()/post() seemed a natural fit, but if we're removing
shutdown_pre() then shudown_wait() works.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Christoph Hellwig]

On Thu, Dec 14, 2023 at 04:03:56PM -0800, Jeremy Allison wrote:
> From: Tanjore Suresh <tansuresh@google.com>
> 
> This changes the bus driver interface with additional entry points
> to enable devices to implement asynchronous shutdown. The existing
> synchronous interface to shutdown is unmodified and retained for
> backward compatibility. shutdown_pre is only called if a matching
> shutdown_post function is also registered, otherwise the synchronous
> synchronous interface is used.
> 
> This changes the common device shutdown code to enable devices to
> participate in asynchronous shutdown implementation.

Is there any reason to have a separate shutdown_pre method?
Especially with all the method wrapping in the driver core, yet
another method just keeps confusing everyone.

And on the post side, might shutdown_wait be a better name to
describe the operation, but I'm open to opinions.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Greg KH]

On Thu, Dec 14, 2023 at 04:03:56PM -0800, Jeremy Allison wrote:
> From: Tanjore Suresh <tansuresh@google.com>
> 
> This changes the bus driver interface with additional entry points
> to enable devices to implement asynchronous shutdown. The existing
> synchronous interface to shutdown is unmodified and retained for
> backward compatibility. shutdown_pre is only called if a matching
> shutdown_post function is also registered, otherwise the synchronous
> synchronous interface is used.
> 
> This changes the common device shutdown code to enable devices to
> participate in asynchronous shutdown implementation.
> 
> Signed-off-by: Tanjore Suresh <tansuresh@google.com>
> Signed-off-by: Jeremy Allison <jallison@ciq.com>

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 1/3] driver core: Support asynchronous driver shutdown

[Jeremy Allison]

From: Tanjore Suresh <tansuresh@google.com>

This changes the bus driver interface with additional entry points
to enable devices to implement asynchronous shutdown. The existing
synchronous interface to shutdown is unmodified and retained for
backward compatibility. shutdown_pre is only called if a matching
shutdown_post function is also registered, otherwise the synchronous
synchronous interface is used.

This changes the common device shutdown code to enable devices to
participate in asynchronous shutdown implementation.

Signed-off-by: Tanjore Suresh <tansuresh@google.com>
Signed-off-by: Jeremy Allison <jallison@ciq.com>
---
 drivers/base/core.c        | 41 +++++++++++++++++++++++++++++++++++++-
 include/linux/device/bus.h | 11 ++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67ba592afc77..a842d402a088 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4725,6 +4725,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
 void device_shutdown(void)
 {
 	struct device *dev, *parent;
+	LIST_HEAD(async_shutdown_list);
 
 	wait_for_device_probe();
 	device_block_probing();
@@ -4769,7 +4770,16 @@ void device_shutdown(void)
 				dev_info(dev, "shutdown_pre\n");
 			dev->class->shutdown_pre(dev);
 		}
-		if (dev->bus && dev->bus->shutdown) {
+
+		/* Only call shutdown_pre if a shutdown_post is also defined. */
+		if (dev->bus && dev->bus->shutdown_pre &&
+				dev->bus->shutdown_post) {
+			if (initcall_debug)
+				dev_info(dev, "shutdown_pre\n");
+			dev->bus->shutdown_pre(dev);
+			list_add(&dev->kobj.entry,
+				&async_shutdown_list);
+		} else if (dev->bus && dev->bus->shutdown) {
 			if (initcall_debug)
 				dev_info(dev, "shutdown\n");
 			dev->bus->shutdown(dev);
@@ -4789,6 +4799,35 @@ void device_shutdown(void)
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+
+	/*
+	 * Second pass spin for only devices, that have configured
+	 * Asynchronous shutdown.
+	 */
+	while (!list_empty(&async_shutdown_list)) {
+		dev = list_entry(async_shutdown_list.next, struct device,
+				kobj.entry);
+		parent = get_device(dev->parent);
+		get_device(dev);
+		/*
+		 * Make sure the device is off the  list
+		 */
+		list_del_init(&dev->kobj.entry);
+		if (parent)
+			device_lock(parent);
+		device_lock(dev);
+		if (dev->bus && dev->bus->shutdown_post) {
+			if (initcall_debug)
+				dev_info(dev,
+				"shutdown_post called\n");
+			dev->bus->shutdown_post(dev);
+		}
+		device_unlock(dev);
+		if (parent)
+			device_unlock(parent);
+		put_device(dev);
+		put_device(parent);
+	}
 }
 
 /*
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index ae10c4322754..d49dae1a280c 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -48,6 +48,15 @@ struct fwnode_handle;
  *		will never get called until they do.
  * @remove:	Called when a device removed from this bus.
  * @shutdown:	Called at shut-down time to quiesce the device.
+ * @shutdown_pre:	Called at the shutdown-time to start the shutdown
+ *			process on the device. This entry point will be called
+ *			only when the bus driver has indicated it would like
+ *			to participate in asynchronous shutdown completion
+ *			and has also defined a shutdown_post function.
+ * @shutdown_post:	Called at shutdown-time  to complete the shutdown
+ *			process of the device. This entry point will be called
+ *			only when the bus drive has indicated it would like to
+ *			participate in the asynchronous shutdown completion.
  *
  * @online:	Called to put the device back online (after offlining it).
  * @offline:	Called to put the device offline for hot-removal. May fail.
@@ -90,6 +99,8 @@ struct bus_type {
 	void (*sync_state)(struct device *dev);
 	void (*remove)(struct device *dev);
 	void (*shutdown)(struct device *dev);
+	void (*shutdown_pre)(struct device *dev);
+	void (*shutdown_post)(struct device *dev);
 
 	int (*online)(struct device *dev);
 	int (*offline)(struct device *dev);
-- 
2.39.3

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Bart Van Assche]

On 12/12/23 10:09, Jeremy Allison wrote:
> From: Tanjore Suresh <tansuresh@google.com>
> 
> This changes the bus driver interface with additional entry points
> to enable devices to implement asynchronous shutdown. The existing
> synchronous interface to shutdown is unmodified and retained for
> backward compatibility.
> 
> This changes the common device shutdown code to enable devices to
> participate in asynchronous shutdown implementation.
> 
> Signed-off-by: Tanjore Suresh <tansuresh@google.com>
> ---
>   drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
>   include/linux/device/bus.h | 10 ++++++++++
>   2 files changed, 48 insertions(+), 1 deletion(-)

 From the MAINTAINERS file:

DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS
M:      Greg Kroah-Hartman <gregkh@linuxfoundation.org>
R:      "Rafael J. Wysocki" <rafael@kernel.org>
S:      Supported
T:      git 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
F:      Documentation/core-api/kobject.rst
F:      drivers/base/
F:      fs/debugfs/
F:      fs/sysfs/
F:      include/linux/debugfs.h
F:      include/linux/fwnode.h
F:      include/linux/kobj*
F:      include/linux/property.h
F:      lib/kobj*

Please Cc the maintainers of the modified files when posting kernel patches.

Thanks,

Bart.

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Jeremy Allison]

Good catch. Thanks. I'll fix that and re-submit.

On Wed, Dec 13, 2023 at 5:59 AM Sagi Grimberg <sagi@grimberg.me> wrote:
>
> > From: Tanjore Suresh <tansuresh@google.com>
> >
> > This changes the bus driver interface with additional entry points
> > to enable devices to implement asynchronous shutdown. The existing
> > synchronous interface to shutdown is unmodified and retained for
> > backward compatibility.
> >
> > This changes the common device shutdown code to enable devices to
> > participate in asynchronous shutdown implementation.
> >
> > Signed-off-by: Tanjore Suresh <tansuresh@google.com>
> > ---
> >   drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
> >   include/linux/device/bus.h | 10 ++++++++++
> >   2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 67ba592afc77..d9745822fb50 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -4725,6 +4725,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
> >   void device_shutdown(void)
> >   {
> >       struct device *dev, *parent;
> > +     LIST_HEAD(async_shutdown_list);
> >
> >       wait_for_device_probe();
> >       device_block_probing();
> > @@ -4769,7 +4770,14 @@ void device_shutdown(void)
> >                               dev_info(dev, "shutdown_pre\n");
> >                       dev->class->shutdown_pre(dev);
> >               }
> > -             if (dev->bus && dev->bus->shutdown) {
> > +
> > +             if (dev->bus && dev->bus->shutdown_pre) {
>
> I'm assuming that there is no shutdown_pre without a shutdown_post
> paired with it, so I think the code should verify that.
>
> > +                     if (initcall_debug)
> > +                             dev_info(dev, "shutdown_pre\n");
> > +                     dev->bus->shutdown_pre(dev);
> > +                     list_add(&dev->kobj.entry,
> > +                             &async_shutdown_list);
> > +             } else if (dev->bus && dev->bus->shutdown) {
> >                       if (initcall_debug)
> >                               dev_info(dev, "shutdown\n");
> >                       dev->bus->shutdown(dev);
> > @@ -4789,6 +4797,35 @@ void device_shutdown(void)
> >               spin_lock(&devices_kset->list_lock);
> >       }
> >       spin_unlock(&devices_kset->list_lock);
> > +
> > +     /*
> > +      * Second pass spin for only devices, that have configured
> > +      * Asynchronous shutdown.
> > +      */
> > +     while (!list_empty(&async_shutdown_list)) {
> > +             dev = list_entry(async_shutdown_list.next, struct device,
> > +                             kobj.entry);
> > +             parent = get_device(dev->parent);
> > +             get_device(dev);
> > +             /*
> > +              * Make sure the device is off the  list
> > +              */
> > +             list_del_init(&dev->kobj.entry);
> > +             if (parent)
> > +                     device_lock(parent);
> > +             device_lock(dev);
> > +             if (dev->bus && dev->bus->shutdown_post) {
> > +                     if (initcall_debug)
> > +                             dev_info(dev,
> > +                             "shutdown_post called\n");
> > +                     dev->bus->shutdown_post(dev);
> > +             }
> > +             device_unlock(dev);
> > +             if (parent)
> > +                     device_unlock(parent);
> > +             put_device(dev);
> > +             put_device(parent);
> > +     }
> >   }
> >
> >   /*
> > diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> > index ae10c4322754..cbcb001f6336 100644
> > --- a/include/linux/device/bus.h
> > +++ b/include/linux/device/bus.h
> > @@ -48,6 +48,14 @@ struct fwnode_handle;
> >    *          will never get called until they do.
> >    * @remove: Called when a device removed from this bus.
> >    * @shutdown:       Called at shut-down time to quiesce the device.
> > + * @shutdown_pre:    Called at the shutdown-time to start the shutdown
> > + *                   process on the device. This entry point will be called
> > + *                   only when the bus driver has indicated it would like
> > + *                   to participate in asynchronous shutdown completion.
> > + * @shutdown_post:   Called at shutdown-time  to complete the shutdown
> > + *                   process of the device. This entry point will be called
> > + *                   only when the bus drive has indicated it would like to
> > + *                   participate in the asynchronous shutdown completion.
> >    *
> >    * @online: Called to put the device back online (after offlining it).
> >    * @offline:        Called to put the device offline for hot-removal. May fail.
> > @@ -90,6 +98,8 @@ struct bus_type {
> >       void (*sync_state)(struct device *dev);
> >       void (*remove)(struct device *dev);
> >       void (*shutdown)(struct device *dev);
> > +     void (*shutdown_pre)(struct device *dev);
> > +     void (*shutdown_post)(struct device *dev);
> >
> >       int (*online)(struct device *dev);
> >       int (*offline)(struct device *dev);

Re: [PATCH 1/3] driver core: Support asynchronous driver shutdown

[Sagi Grimberg]

> From: Tanjore Suresh <tansuresh@google.com>
> 
> This changes the bus driver interface with additional entry points
> to enable devices to implement asynchronous shutdown. The existing
> synchronous interface to shutdown is unmodified and retained for
> backward compatibility.
> 
> This changes the common device shutdown code to enable devices to
> participate in asynchronous shutdown implementation.
> 
> Signed-off-by: Tanjore Suresh <tansuresh@google.com>
> ---
>   drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
>   include/linux/device/bus.h | 10 ++++++++++
>   2 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 67ba592afc77..d9745822fb50 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4725,6 +4725,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
>   void device_shutdown(void)
>   {
>   	struct device *dev, *parent;
> +	LIST_HEAD(async_shutdown_list);
>   
>   	wait_for_device_probe();
>   	device_block_probing();
> @@ -4769,7 +4770,14 @@ void device_shutdown(void)
>   				dev_info(dev, "shutdown_pre\n");
>   			dev->class->shutdown_pre(dev);
>   		}
> -		if (dev->bus && dev->bus->shutdown) {
> +
> +		if (dev->bus && dev->bus->shutdown_pre) {

I'm assuming that there is no shutdown_pre without a shutdown_post
paired with it, so I think the code should verify that.

> +			if (initcall_debug)
> +				dev_info(dev, "shutdown_pre\n");
> +			dev->bus->shutdown_pre(dev);
> +			list_add(&dev->kobj.entry,
> +				&async_shutdown_list);
> +		} else if (dev->bus && dev->bus->shutdown) {
>   			if (initcall_debug)
>   				dev_info(dev, "shutdown\n");
>   			dev->bus->shutdown(dev);
> @@ -4789,6 +4797,35 @@ void device_shutdown(void)
>   		spin_lock(&devices_kset->list_lock);
>   	}
>   	spin_unlock(&devices_kset->list_lock);
> +
> +	/*
> +	 * Second pass spin for only devices, that have configured
> +	 * Asynchronous shutdown.
> +	 */
> +	while (!list_empty(&async_shutdown_list)) {
> +		dev = list_entry(async_shutdown_list.next, struct device,
> +				kobj.entry);
> +		parent = get_device(dev->parent);
> +		get_device(dev);
> +		/*
> +		 * Make sure the device is off the  list
> +		 */
> +		list_del_init(&dev->kobj.entry);
> +		if (parent)
> +			device_lock(parent);
> +		device_lock(dev);
> +		if (dev->bus && dev->bus->shutdown_post) {
> +			if (initcall_debug)
> +				dev_info(dev,
> +				"shutdown_post called\n");
> +			dev->bus->shutdown_post(dev);
> +		}
> +		device_unlock(dev);
> +		if (parent)
> +			device_unlock(parent);
> +		put_device(dev);
> +		put_device(parent);
> +	}
>   }
>   
>   /*
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index ae10c4322754..cbcb001f6336 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -48,6 +48,14 @@ struct fwnode_handle;
>    *		will never get called until they do.
>    * @remove:	Called when a device removed from this bus.
>    * @shutdown:	Called at shut-down time to quiesce the device.
> + * @shutdown_pre:	Called at the shutdown-time to start the shutdown
> + *			process on the device. This entry point will be called
> + *			only when the bus driver has indicated it would like
> + *			to participate in asynchronous shutdown completion.
> + * @shutdown_post:	Called at shutdown-time  to complete the shutdown
> + *			process of the device. This entry point will be called
> + *			only when the bus drive has indicated it would like to
> + *			participate in the asynchronous shutdown completion.
>    *
>    * @online:	Called to put the device back online (after offlining it).
>    * @offline:	Called to put the device offline for hot-removal. May fail.
> @@ -90,6 +98,8 @@ struct bus_type {
>   	void (*sync_state)(struct device *dev);
>   	void (*remove)(struct device *dev);
>   	void (*shutdown)(struct device *dev);
> +	void (*shutdown_pre)(struct device *dev);
> +	void (*shutdown_post)(struct device *dev);
>   
>   	int (*online)(struct device *dev);
>   	int (*offline)(struct device *dev);

[PATCH 1/3] driver core: Support asynchronous driver shutdown

[Jeremy Allison]

From: Tanjore Suresh <tansuresh@google.com>

This changes the bus driver interface with additional entry points
to enable devices to implement asynchronous shutdown. The existing
synchronous interface to shutdown is unmodified and retained for
backward compatibility.

This changes the common device shutdown code to enable devices to
participate in asynchronous shutdown implementation.

Signed-off-by: Tanjore Suresh <tansuresh@google.com>
---
 drivers/base/core.c        | 39 +++++++++++++++++++++++++++++++++++++-
 include/linux/device/bus.h | 10 ++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 67ba592afc77..d9745822fb50 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4725,6 +4725,7 @@ EXPORT_SYMBOL_GPL(device_change_owner);
 void device_shutdown(void)
 {
 	struct device *dev, *parent;
+	LIST_HEAD(async_shutdown_list);
 
 	wait_for_device_probe();
 	device_block_probing();
@@ -4769,7 +4770,14 @@ void device_shutdown(void)
 				dev_info(dev, "shutdown_pre\n");
 			dev->class->shutdown_pre(dev);
 		}
-		if (dev->bus && dev->bus->shutdown) {
+
+		if (dev->bus && dev->bus->shutdown_pre) {
+			if (initcall_debug)
+				dev_info(dev, "shutdown_pre\n");
+			dev->bus->shutdown_pre(dev);
+			list_add(&dev->kobj.entry,
+				&async_shutdown_list);
+		} else if (dev->bus && dev->bus->shutdown) {
 			if (initcall_debug)
 				dev_info(dev, "shutdown\n");
 			dev->bus->shutdown(dev);
@@ -4789,6 +4797,35 @@ void device_shutdown(void)
 		spin_lock(&devices_kset->list_lock);
 	}
 	spin_unlock(&devices_kset->list_lock);
+
+	/*
+	 * Second pass spin for only devices, that have configured
+	 * Asynchronous shutdown.
+	 */
+	while (!list_empty(&async_shutdown_list)) {
+		dev = list_entry(async_shutdown_list.next, struct device,
+				kobj.entry);
+		parent = get_device(dev->parent);
+		get_device(dev);
+		/*
+		 * Make sure the device is off the  list
+		 */
+		list_del_init(&dev->kobj.entry);
+		if (parent)
+			device_lock(parent);
+		device_lock(dev);
+		if (dev->bus && dev->bus->shutdown_post) {
+			if (initcall_debug)
+				dev_info(dev,
+				"shutdown_post called\n");
+			dev->bus->shutdown_post(dev);
+		}
+		device_unlock(dev);
+		if (parent)
+			device_unlock(parent);
+		put_device(dev);
+		put_device(parent);
+	}
 }
 
 /*
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index ae10c4322754..cbcb001f6336 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -48,6 +48,14 @@ struct fwnode_handle;
  *		will never get called until they do.
  * @remove:	Called when a device removed from this bus.
  * @shutdown:	Called at shut-down time to quiesce the device.
+ * @shutdown_pre:	Called at the shutdown-time to start the shutdown
+ *			process on the device. This entry point will be called
+ *			only when the bus driver has indicated it would like
+ *			to participate in asynchronous shutdown completion.
+ * @shutdown_post:	Called at shutdown-time  to complete the shutdown
+ *			process of the device. This entry point will be called
+ *			only when the bus drive has indicated it would like to
+ *			participate in the asynchronous shutdown completion.
  *
  * @online:	Called to put the device back online (after offlining it).
  * @offline:	Called to put the device offline for hot-removal. May fail.
@@ -90,6 +98,8 @@ struct bus_type {
 	void (*sync_state)(struct device *dev);
 	void (*remove)(struct device *dev);
 	void (*shutdown)(struct device *dev);
+	void (*shutdown_pre)(struct device *dev);
+	void (*shutdown_post)(struct device *dev);
 
 	int (*online)(struct device *dev);
 	int (*offline)(struct device *dev);
-- 
2.39.3