| Message ID | 20240325123444.3031851-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | ACPI: scan: A few ad-hoc cleanups | expand |
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!
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!
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); } /**
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(-)