From c1013ff7a5472db637c56bb6237f8343398c03a7 Mon Sep 17 00:00:00 2001

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Date: Thu, 14 Jan 2021 19:46:47 +0100

Subject: [PATCH] ACPI: scan: Rearrange memory allocation in acpi_device_add()

Git-commit: c1013ff7a5472db637c56bb6237f8343398c03a7

Patch-mainline: v5.12-rc1

References: git-fixes

The upfront allocation of new_bus_id is done to avoid allocating

memory under acpi_device_lock, but it doesn't really help,

because (1) it leads to many unnecessary memory allocations for

_ADR devices, (2) kstrdup_const() is run under that lock anyway and

(3) it complicates the code.

Rearrange acpi_device_add() to allocate memory for a new struct

acpi_device_bus_id instance only when necessary, eliminate a redundant

local variable from it and reduce the number of labels in there.

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Takashi Iwai <tiwai@suse.de>

---

 drivers/acpi/scan.c | 57 +++++++++++++++++++++------------------------

 1 file changed, 26 insertions(+), 31 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c

index 1db063b02f63..0fb1811772b5 100644

--- a/drivers/acpi/scan.c

+++ b/drivers/acpi/scan.c

@@ -623,12 +623,23 @@ void acpi_bus_put_acpi_device(struct acpi_device *adev)

 	put_device(&adev->dev);

 }

+static struct acpi_device_bus_id *acpi_device_bus_id_match(const char *dev_id)

+{

+	struct acpi_device_bus_id *acpi_device_bus_id;

+

+	/* Find suitable bus_id and instance number in acpi_bus_id_list. */

+	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {

+		if (!strcmp(acpi_device_bus_id->bus_id, dev_id))

+			return acpi_device_bus_id;

+	}

+	return NULL;

+}

+

 int acpi_device_add(struct acpi_device *device,

 		    void (*release)(struct device *))

 {

+	struct acpi_device_bus_id *acpi_device_bus_id;

 	int result;

-	struct acpi_device_bus_id *acpi_device_bus_id, *new_bus_id;

-	int found = 0;

 	if (device->handle) {

 		acpi_status status;

@@ -654,38 +665,26 @@ int acpi_device_add(struct acpi_device *device,

 	INIT_LIST_HEAD(&device->del_list);

 	mutex_init(&device->physical_node_lock);

-	new_bus_id = kzalloc(sizeof(struct acpi_device_bus_id), GFP_KERNEL);

-	if (!new_bus_id) {

-		pr_err(PREFIX "Memory allocation error\n");

-		result = -ENOMEM;

-		goto err_detach;

-	}

-

 	mutex_lock(&acpi_device_lock);

-	/*

-	 * Find suitable bus_id and instance number in acpi_bus_id_list

-	 * If failed, create one and link it into acpi_bus_id_list

-	 */

-	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {

-		if (!strcmp(acpi_device_bus_id->bus_id,

-			    acpi_device_hid(device))) {

-			acpi_device_bus_id->instance_no++;

-			found = 1;

-			kfree(new_bus_id);

-			break;

+

+	acpi_device_bus_id = acpi_device_bus_id_match(acpi_device_hid(device));

+	if (acpi_device_bus_id) {

+		acpi_device_bus_id->instance_no++;

+	} else {

+		acpi_device_bus_id = kzalloc(sizeof(*acpi_device_bus_id),

+					     GFP_KERNEL);

+		if (!acpi_device_bus_id) {

+			result = -ENOMEM;

+			goto err_unlock;

 		}

-	}

-	if (!found) {

-		acpi_device_bus_id = new_bus_id;

 		acpi_device_bus_id->bus_id =

 			kstrdup_const(acpi_device_hid(device), GFP_KERNEL);

 		if (!acpi_device_bus_id->bus_id) {

-			pr_err(PREFIX "Memory allocation error for bus id\n");

+			kfree(acpi_device_bus_id);

 			result = -ENOMEM;

-			goto err_free_new_bus_id;

+			goto err_unlock;

 		}

-		acpi_device_bus_id->instance_no = 0;

 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);

 	}

 	dev_set_name(&device->dev, "%s:%02x", acpi_device_bus_id->bus_id, acpi_device_bus_id->instance_no);

@@ -720,13 +719,9 @@ int acpi_device_add(struct acpi_device *device,

 		list_del(&device->node);

 	list_del(&device->wakeup_list);

- err_free_new_bus_id:

-	if (!found)

-		kfree(new_bus_id);

-

+ err_unlock:

 	mutex_unlock(&acpi_device_lock);

- err_detach:

 	acpi_detach_data(device->handle, acpi_scan_drop_device);

 	return result;

 }

-- 

2.26.2