Hi Alex-san, Thank you very much for your continuous effort. I have one comment about the new design. Please see below. >> I found another problem in pci_hp_register() that would be more complex
to fix than above-mentioned problem. Here is a pci_hp_register() with all of your patch applied. int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr, const char *name) { (snip...) /* * No problems if we call this interface from both ACPI_PCI_SLOT * driver and call it here again. If we've already created the * pci_slot, the interface will simply bump the refcount. */ pci_slot = pci_create_slot(bus, slot_nr, name); if (IS_ERR(pci_slot)) return PTR_ERR(pci_slot); if (pci_slot->hotplug) { dbg("%s: already claimed\n", __func__); pci_destroy_slot(pci_slot); return -EBUSY; } slot->pci_slot = pci_slot; pci_slot->hotplug = slot; /* * Allow pcihp drivers to override the ACPI_PCI_SLOT name. */ if (strcmp(kobject_name(&pci_slot->kobj), name)) { result = kobject_rename(&pci_slot->kobj, name); if (result) { pci_destroy_slot(pci_slot); return result; } } (snip...) } If name duplication was detected in pci_create_slot(), it renames the slot name like 'N-1' and return successfully. Even though slot's kobject name was registered as 'N-1', 'name' array still have 'N' at this point. So the following 'if' statement becomes true unexpectedly. /* * Allow pcihp drivers to override the ACPI_PCI_SLOT name. */ if (strcmp(kobject_name(&pci_slot->kobj), name)) { Then pci_hp_register() attempt to rename the slot name with 'N' again by calling kobject_rename(), but it fails because there already exists kobject with name 'N'. As a result, pci_hp_register() will fail.Yes, you are right, that is a problem. I've taken the following approach: - the above code is providing a mechanism to allow a _hotplug_ driver to override a _detection_ driver slot name. - in other words, we only have to worry about the case when a _detection_ driver was loaded before a _hotplug_ driver. - we can ignore the case where another _hotplug_ driver was loaded first, because we'll already return -EBUSY. - so, to figure out if a _detection_ driver has already been loaded, we check to see if the pci_dev already has a valid pci_slot pointer.
We need to take into account that the hotplug slot can be empty. In this case, we cannot do this check because pci_dev doesn't exist, I think.
- if yes, then we later check to see if the existing slot name matches the requested slot name from the hotplug driver. - if the hotplug driver is requesting a different name, then we use a new interface, pci_rename_slot() which will safely attempt to rename the slot without name collision. I'll send out the patch set soon, it would be great if you could test it for me, since I don't have systems with duplicate slot names.
Sure. P.S. I also don't have the system with duplicate slot names. So I use the debug patch that emulates this kind of system for testing. Thanks, Kenji Kaneshige -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
