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