[v1,11/12] PM: sleep: Move devices to new lists earlier in each suspend phase

Message ID 3476604.QJadu78ljV@kreacher
State New
Headers
Series PM: sleep: Fix up suspend stats handling and clean up core code |

Commit Message

Rafael J. Wysocki Jan. 22, 2024, 11:42 a.m. UTC
  From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

During a system-wide suspend of devices, dpm_noirq_suspend_devices(),
dpm_suspend_late() and dpm_suspend() move devices from one list to
another.  They do it with each device after its PM callback in the
given suspend phase has run or has been scheduled for asynchronous
execution, in case it is deleted from the current list in the
meantime.

However, devices can be moved to a new list before invoking their PM
callbacks (which usually is the case for the devices whose callbacks
are executed asynchronously anyway), because doing so does not affect
the ordering of that list.  In either case, each device is moved to
the new list after the previous device has been moved to it or gone
away, and if a device is removed, it does not matter which list it is
in at that point, because deleting an entry from a list does not change
the ordering of the other entries in it.

Accordingly, modify the functions mentioned above to move devices to
new lists without waiting for their PM callbacks to run regardless of
whether or not they run asynchronously.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/main.c |   24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)
  

Comments

Stanislaw Gruszka Jan. 25, 2024, 8 a.m. UTC | #1
On Mon, Jan 22, 2024 at 12:42:46PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> During a system-wide suspend of devices, dpm_noirq_suspend_devices(),
> dpm_suspend_late() and dpm_suspend() move devices from one list to
> another.  They do it with each device after its PM callback in the
> given suspend phase has run or has been scheduled for asynchronous
> execution, in case it is deleted from the current list in the
> meantime.
> 
> However, devices can be moved to a new list before invoking their PM
> callbacks (which usually is the case for the devices whose callbacks
> are executed asynchronously anyway), because doing so does not affect
> the ordering of that list.  In either case, each device is moved to
> the new list after the previous device has been moved to it or gone
> away, and if a device is removed, it does not matter which list it is
> in at that point, because deleting an entry from a list does not change
> the ordering of the other entries in it.
> 
> Accordingly, modify the functions mentioned above to move devices to
> new lists without waiting for their PM callbacks to run regardless of
> whether or not they run asynchronously.
> 
> No intentional functional impact.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>

> ---
>  drivers/base/power/main.c |   24 +++---------------------
>  1 file changed, 3 insertions(+), 21 deletions(-)
> 
> Index: linux-pm/drivers/base/power/main.c
> ===================================================================
> --- linux-pm.orig/drivers/base/power/main.c
> +++ linux-pm/drivers/base/power/main.c
> @@ -1304,18 +1304,12 @@ static int dpm_noirq_suspend_devices(pm_
>  	while (!list_empty(&dpm_late_early_list)) {
>  		struct device *dev = to_device(dpm_late_early_list.prev);
>  
> +		list_move(&dev->power.entry, &dpm_noirq_list);
>  		get_device(dev);
>  		mutex_unlock(&dpm_list_mtx);
>  
>  		error = device_suspend_noirq(dev);
>  
> -		mutex_lock(&dpm_list_mtx);
> -
> -		if (!error && !list_empty(&dev->power.entry))
> -			list_move(&dev->power.entry, &dpm_noirq_list);
> -
> -		mutex_unlock(&dpm_list_mtx);
> -
>  		put_device(dev);
>  
>  		mutex_lock(&dpm_list_mtx);
> @@ -1486,19 +1480,13 @@ int dpm_suspend_late(pm_message_t state)
>  	while (!list_empty(&dpm_suspended_list)) {
>  		struct device *dev = to_device(dpm_suspended_list.prev);
>  
> +		list_move(&dev->power.entry, &dpm_late_early_list);
>  		get_device(dev);
>  
>  		mutex_unlock(&dpm_list_mtx);
>  
>  		error = device_suspend_late(dev);
>  
> -		mutex_lock(&dpm_list_mtx);
> -
> -		if (!list_empty(&dev->power.entry))
> -			list_move(&dev->power.entry, &dpm_late_early_list);
> -
> -		mutex_unlock(&dpm_list_mtx);
> -
>  		put_device(dev);
>  
>  		mutex_lock(&dpm_list_mtx);
> @@ -1763,19 +1751,13 @@ int dpm_suspend(pm_message_t state)
>  	while (!list_empty(&dpm_prepared_list)) {
>  		struct device *dev = to_device(dpm_prepared_list.prev);
>  
> +		list_move(&dev->power.entry, &dpm_suspended_list);
>  		get_device(dev);
>  
>  		mutex_unlock(&dpm_list_mtx);
>  
>  		error = device_suspend(dev);
>  
> -		mutex_lock(&dpm_list_mtx);
> -
> -		if (!error && !list_empty(&dev->power.entry))
> -			list_move(&dev->power.entry, &dpm_suspended_list);
> -
> -		mutex_unlock(&dpm_list_mtx);
> -
>  		put_device(dev);
>  
>  		mutex_lock(&dpm_list_mtx);
> 
> 
> 
>
  

Patch

Index: linux-pm/drivers/base/power/main.c
===================================================================
--- linux-pm.orig/drivers/base/power/main.c
+++ linux-pm/drivers/base/power/main.c
@@ -1304,18 +1304,12 @@  static int dpm_noirq_suspend_devices(pm_
 	while (!list_empty(&dpm_late_early_list)) {
 		struct device *dev = to_device(dpm_late_early_list.prev);
 
+		list_move(&dev->power.entry, &dpm_noirq_list);
 		get_device(dev);
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_suspend_noirq(dev);
 
-		mutex_lock(&dpm_list_mtx);
-
-		if (!error && !list_empty(&dev->power.entry))
-			list_move(&dev->power.entry, &dpm_noirq_list);
-
-		mutex_unlock(&dpm_list_mtx);
-
 		put_device(dev);
 
 		mutex_lock(&dpm_list_mtx);
@@ -1486,19 +1480,13 @@  int dpm_suspend_late(pm_message_t state)
 	while (!list_empty(&dpm_suspended_list)) {
 		struct device *dev = to_device(dpm_suspended_list.prev);
 
+		list_move(&dev->power.entry, &dpm_late_early_list);
 		get_device(dev);
 
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_suspend_late(dev);
 
-		mutex_lock(&dpm_list_mtx);
-
-		if (!list_empty(&dev->power.entry))
-			list_move(&dev->power.entry, &dpm_late_early_list);
-
-		mutex_unlock(&dpm_list_mtx);
-
 		put_device(dev);
 
 		mutex_lock(&dpm_list_mtx);
@@ -1763,19 +1751,13 @@  int dpm_suspend(pm_message_t state)
 	while (!list_empty(&dpm_prepared_list)) {
 		struct device *dev = to_device(dpm_prepared_list.prev);
 
+		list_move(&dev->power.entry, &dpm_suspended_list);
 		get_device(dev);
 
 		mutex_unlock(&dpm_list_mtx);
 
 		error = device_suspend(dev);
 
-		mutex_lock(&dpm_list_mtx);
-
-		if (!error && !list_empty(&dev->power.entry))
-			list_move(&dev->power.entry, &dpm_suspended_list);
-
-		mutex_unlock(&dpm_list_mtx);
-
 		put_device(dev);
 
 		mutex_lock(&dpm_list_mtx);