While looking for something else in the scan.c I noticed the room
of improvement. Hence this series. Also bus.c patches, which some
how were related to my research, but I think are independent from
the scan.c improvements.
Andy Shevchenko (7):
ACPI: bus: Make container_of() no-op where it makes sense
ACPI: bus: Don't use "proxy" headers
ACPI: scan: Replace infinite for-loop with finite while-loop
ACPI: scan: Use list_first_entry_or_null() in acpi_device_hid()
ACPI: scan: Move misleading comment to acpi_dma_configure_id()
ACPI: scan: Use standard error checking pattern
ACPI: scan: Introduce typedef:s for struct acpi_hotplug_context
members
drivers/acpi/dock.c | 48 +++++++++++++++--------------------------
drivers/acpi/scan.c | 42 +++++++++++++++++-------------------
include/acpi/acpi_bus.h | 28 +++++++++++++++---------
3 files changed, 55 insertions(+), 63 deletions(-)
--
2.43.0.rc1.1.gbec44491f096
Update header inclusions to follow IWYU (Include What You Use)
principle.
Signed-off-by: Andy Shevchenko <[email protected]>
---
include/acpi/acpi_bus.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index cdee38ef9ba5..acb62d1d3306 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -9,8 +9,13 @@
#ifndef __ACPI_BUS_H__
#define __ACPI_BUS_H__
+#include <linux/completion.h>
+#include <linux/container_of.h>
#include <linux/device.h>
+#include <linux/kobject.h>
+#include <linux/mutex.h>
#include <linux/property.h>
+#include <linux/types.h>
struct acpi_handle_list {
u32 count;
--
2.43.0.rc1.1.gbec44491f096
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 <[email protected]>
---
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
On 3/25/24 5:32 AM, Andy Shevchenko wrote:
> While looking for something else in the scan.c I noticed the room
> of improvement. Hence this series. Also bus.c patches, which some
> how were related to my research, but I think are independent from
> the scan.c improvements.
Changes looks good to me.
Reviewed-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
> Andy Shevchenko (7):
> ACPI: bus: Make container_of() no-op where it makes sense
> ACPI: bus: Don't use "proxy" headers
> ACPI: scan: Replace infinite for-loop with finite while-loop
> ACPI: scan: Use list_first_entry_or_null() in acpi_device_hid()
> ACPI: scan: Move misleading comment to acpi_dma_configure_id()
> ACPI: scan: Use standard error checking pattern
> ACPI: scan: Introduce typedef:s for struct acpi_hotplug_context
> members
>
> drivers/acpi/dock.c | 48 +++++++++++++++--------------------------
> drivers/acpi/scan.c | 42 +++++++++++++++++-------------------
> include/acpi/acpi_bus.h | 28 +++++++++++++++---------
> 3 files changed, 55 insertions(+), 63 deletions(-)
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On Mon, Mar 25, 2024 at 1:34 PM Andy Shevchenko
<[email protected]> 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 <[email protected]>
> ---
> 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
> <[email protected]> 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