[PATCH v1 3/7] ACPI: scan: Replace infinite for-loop with finite while-loop

Andy Shevchenko posted 7 patches 1 year, 10 months ago
[PATCH v1 3/7] ACPI: scan: Replace infinite for-loop with finite while-loop
Posted by Andy Shevchenko 1 year, 10 months ago
The infinite loops is harder to understand (as one has to go
over the body in order to find main exit conditional) and it's
more verbose than usual approach with a while-loop.

Note, we may not use list_for_each_entry_safe() as there is locking
involved and the saved pointer may become invalid behind our back.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/scan.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 7c157bf92695..5e4118970285 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -530,15 +530,10 @@ static DEFINE_MUTEX(acpi_device_del_lock);
 
 static void acpi_device_del_work_fn(struct work_struct *work_not_used)
 {
-	for (;;) {
-		struct acpi_device *adev;
+	struct acpi_device *adev;
 
-		mutex_lock(&acpi_device_del_lock);
-
-		if (list_empty(&acpi_device_del_list)) {
-			mutex_unlock(&acpi_device_del_lock);
-			break;
-		}
+	mutex_lock(&acpi_device_del_lock);
+	while (!list_empty(&acpi_device_del_list)) {
 		adev = list_first_entry(&acpi_device_del_list,
 					struct acpi_device, del_list);
 		list_del(&adev->del_list);
@@ -555,7 +550,10 @@ static void acpi_device_del_work_fn(struct work_struct *work_not_used)
 		 */
 		acpi_power_transition(adev, ACPI_STATE_D3_COLD);
 		acpi_dev_put(adev);
+
+		mutex_lock(&acpi_device_del_lock);
 	}
+	mutex_unlock(&acpi_device_del_lock);
 }
 
 /**
-- 
2.43.0.rc1.1.gbec44491f096
Re: [PATCH v1 3/7] ACPI: scan: Replace infinite for-loop with finite while-loop
Posted by Rafael J. Wysocki 1 year, 10 months ago
On Mon, Mar 25, 2024 at 1:34 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The infinite loops is harder to understand (as one has to go
> over the body in order to find main exit conditional) and it's
> more verbose than usual approach with a while-loop.
>
> Note, we may not use list_for_each_entry_safe() as there is locking
> involved and the saved pointer may become invalid behind our back.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/acpi/scan.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 7c157bf92695..5e4118970285 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -530,15 +530,10 @@ static DEFINE_MUTEX(acpi_device_del_lock);
>
>  static void acpi_device_del_work_fn(struct work_struct *work_not_used)
>  {
> -       for (;;) {
> -               struct acpi_device *adev;
> +       struct acpi_device *adev;
>
> -               mutex_lock(&acpi_device_del_lock);
> -
> -               if (list_empty(&acpi_device_del_list)) {
> -                       mutex_unlock(&acpi_device_del_lock);
> -                       break;
> -               }
> +       mutex_lock(&acpi_device_del_lock);
> +       while (!list_empty(&acpi_device_del_list)) {
>                 adev = list_first_entry(&acpi_device_del_list,
>                                         struct acpi_device, del_list);
>                 list_del(&adev->del_list);
> @@ -555,7 +550,10 @@ static void acpi_device_del_work_fn(struct work_struct *work_not_used)
>                  */
>                 acpi_power_transition(adev, ACPI_STATE_D3_COLD);
>                 acpi_dev_put(adev);
> +
> +               mutex_lock(&acpi_device_del_lock);
>         }
> +       mutex_unlock(&acpi_device_del_lock);
>  }
>
>  /**
> --

I don't quite agree with this one, sorry.

The rest of the series has been applied as 6.10 material.

Thanks!
Re: [PATCH v1 3/7] ACPI: scan: Replace infinite for-loop with finite while-loop
Posted by Andy Shevchenko 1 year, 10 months ago
On Thu, Apr 04, 2024 at 09:22:29PM +0200, Rafael J. Wysocki wrote:
> On Mon, Mar 25, 2024 at 1:34 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:

...

> I don't quite agree with this one, sorry.

No problem.

> The rest of the series has been applied as 6.10 material.

Thank you!

-- 
With Best Regards,
Andy Shevchenko