Received-SPF: pass (google.com: domain of linux-kernel+bounces-132027-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240325123444.3031851-1-andriy.shevchenko@linux.intel.com> <20240325123444.3031851-4-andriy.shevchenko@linux.intel.com>
In-Reply-To: <20240325123444.3031851-4-andriy.shevchenko@linux.intel.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Thu, 4 Apr 2024 21:22:29 +0200
Message-ID: <CAJZ5v0g6bn4k2Sv+SRC1fiDoXU+hZ2iUR6GuL5O1Eny=Pxmh0w@mail.gmail.com>
Subject: Re: [PATCH v1 3/7] ACPI: scan: Replace infinite for-loop with finite while-loop
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>, linux-acpi@vger.kernel.org, 
	linux-kernel@vger.kernel.org, acpica-devel@lists.linux.dev, 
	"Rafael J. Wysocki" <rafael@kernel.org>, Len Brown <lenb@kernel.org>, 
	Robert Moore <robert.moore@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Mar 25, 2024 at 1:34=E2=80=AFPM 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 =3D 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_stru=
ct *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!