[PATCH v1 1/1] ACPI: scan: Use unique number for instance_no

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Current mechanism of incrementing and decrementing plain integer
to get a next free instance_no when creating an ACPI device is fragile.

In case of hot plug event or namespace removal of the device instances
with the low numbers the plain integer counter can't cover the gaps
and become desynchronized with real state of affairs. If during next
hot plug event or namespace injection the new instances of
the devices need to be instantiated, the counter may mistakenly point
to the existing instance_no and kernel will complain:
"sysfs: cannot create duplicate filename '/bus/acpi/devices/XXXX1234:02'"

Replace plain integer approach by using IDA framework.

Fixes: e49bd2dd5a50 ("ACPI: use PNPID:instance_no as bus_id of ACPI device")
Fixes: ca9dc8d42b30 ("ACPI / scan: Fix acpi_bus_id_list bookkeeping")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
 drivers/acpi/internal.h |  4 ++-
 drivers/acpi/scan.c     | 55 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index e6a5d997241c..6fee4f71ba1c 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -9,6 +9,8 @@
 #ifndef _ACPI_INTERNAL_H_
 #define _ACPI_INTERNAL_H_
 
+#include <linux/idr.h>
+
 #define PREFIX "ACPI: "
 
 int early_acpi_osi_init(void);
@@ -98,7 +100,7 @@ extern struct list_head acpi_bus_id_list;
 
 struct acpi_device_bus_id {
 	const char *bus_id;
-	unsigned int instance_no;
+	struct ida no;
 	struct list_head node;
 };
 
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a184529d8fa4..a118a58f7dad 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -468,9 +468,27 @@ static void acpi_device_release(struct device *dev)
 	kfree(acpi_dev);
 }
 
+static int acpi_device_get_instance_no(struct acpi_device *device)
+{
+	const char *p;
+	int result;
+	int error;
+
+	p = strrchr(dev_name(&device->dev), ':');
+	if (!p)
+		return -ENODATA;
+
+	error = kstrtoint(p + 1, 16, &result);
+	if (error)
+		return error;
+
+	return result;
+}
+
 static void acpi_device_del(struct acpi_device *device)
 {
 	struct acpi_device_bus_id *acpi_device_bus_id;
+	int result;
 
 	mutex_lock(&acpi_device_lock);
 	if (device->parent)
@@ -479,9 +497,13 @@ static void acpi_device_del(struct acpi_device *device)
 	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))) {
-			if (acpi_device_bus_id->instance_no > 0)
-				acpi_device_bus_id->instance_no--;
-			else {
+			result = acpi_device_get_instance_no(device);
+			if (result < 0)
+				dev_warn(&device->dev, "Can't get instance no\n");
+			else
+				ida_simple_remove(&acpi_device_bus_id->no, result);
+
+			if (ida_is_empty(&acpi_device_bus_id->no)) {
 				list_del(&acpi_device_bus_id->node);
 				kfree_const(acpi_device_bus_id->bus_id);
 				kfree(acpi_device_bus_id);
@@ -631,6 +653,19 @@ static struct acpi_device_bus_id *acpi_device_bus_id_match(const char *dev_id)
 	return NULL;
 }
 
+static int acpi_device_set_name(struct acpi_device *device,
+				struct acpi_device_bus_id *acpi_device_bus_id)
+{
+	int result;
+
+	result = ida_simple_get(&acpi_device_bus_id->no, 0, 255, GFP_KERNEL);
+	if (result < 0)
+		return result;
+
+	dev_set_name(&device->dev, "%s:%02x", acpi_device_bus_id->bus_id, result);
+	return 0;
+}
+
 int acpi_device_add(struct acpi_device *device,
 		    void (*release)(struct device *))
 {
@@ -665,7 +700,9 @@ int acpi_device_add(struct acpi_device *device,
 
 	acpi_device_bus_id = acpi_device_bus_id_match(acpi_device_hid(device));
 	if (acpi_device_bus_id) {
-		acpi_device_bus_id->instance_no++;
+		result = acpi_device_set_name(device, acpi_device_bus_id);
+		if (result < 0)
+			goto err_unlock;
 	} else {
 		acpi_device_bus_id = kzalloc(sizeof(*acpi_device_bus_id),
 					     GFP_KERNEL);
@@ -681,9 +718,17 @@ int acpi_device_add(struct acpi_device *device,
 			goto err_unlock;
 		}
 
+		ida_init(&acpi_device_bus_id->no);
+
+		result = acpi_device_set_name(device, acpi_device_bus_id);
+		if (result < 0) {
+			ida_destroy(&acpi_device_bus_id->no);
+			kfree(acpi_device_bus_id);
+			goto err_unlock;
+		}
+
 		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);
 
 	if (device->parent)
 		list_add_tail(&device->node, &device->parent->children);
-- 
2.30.1




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux