On Fri, Mar 19, 2021 at 06:00:38PM +0100, Rafael J. Wysocki wrote: > On Fri, Mar 12, 2021 at 5:02 PM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > 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'" > > This is a slightly convoluted way of stating that there is a bug in > acpi_device_del(). Any suggestion how to massage the above? But in the dry end, yes, decrementing is a bug. > Yes, there is one, the instance_no decrementation is clearly incorrect. > > > Replace plain integer approach by using IDA framework. > > Also the general idea of using IDA for the instance numbering is a good one IMO. ... > > - unsigned int instance_no; > > + struct ida no; > > struct ida instance_ida; ? Will rename. ... > > + p = strrchr(dev_name(&device->dev), ':'); > > + if (!p) > > + return -ENODATA; > > + > > + error = kstrtoint(p + 1, 16, &result); > > + if (error) > > + return error; > > + > > + return result; > > I don't like the above at all. > > I would just store the instance number in struct acpi_device_pnp (say). TBH, I simply didn't know which struct to touch and left this one and I also don't like it. Lemme see if acpi_device_pnp is good enough for that. -- With Best Regards, Andy Shevchenko
