Re: [PATCH v2 02/13] PCI: prevent duplicate slot names

From: Kenji Kaneshige
Date: Wed Sep 10 2008 - 22:49:05 EST

Alex Chiang wrote:
Prevent callers of pci_create_slot() from registering slots with
duplicate names. This condition occurs most often when PCI hotplug
drivers are loaded on platforms with broken firmware that assigns
identical names to multiple slots.

We now rename these duplicate slots on behalf of the user.

If firmware assigns the name N to multiple slots, then:

The first registered slot is assigned N
The second registered slot is assigned N-1
The third registered slot is assigned N-2
The Mth registered slot becomes N-M

A side effect of this patch is that the error condition for when
multiple drivers attempt to claim the same slot becomes much more

In other words, the previous error condition returned for
duplicate slot names (-EEXIST) masked the case when multiple
drivers attempted to claim the same slot. Now, the -EBUSY return
makes the true error more obvious.

This is the permanent fix mentioned in earlier commits:

shpchp: Rename duplicate slot name N as N-1, N-2, N-M...

pciehp: Rename duplicate slot name N as N-1, N-2, N-M...

Cc: jbarnes@xxxxxxxxxxxxxxxx
Cc: kristen.c.accardi@xxxxxxxxx
Cc: matthew@xxxxxx
Cc: kaneshige.kenji@xxxxxxxxxxxxxx
Signed-off-by: Alex Chiang <achiang@xxxxxx>

drivers/pci/hotplug/pci_hotplug_core.c | 23 ++++--
drivers/pci/hotplug/pciehp_core.c | 14 ----
drivers/pci/hotplug/shpchp_core.c | 15 ----
drivers/pci/slot.c | 117 +++++++++++++++++++++++++++-----
include/linux/pci.h | 3 +
5 files changed, 117 insertions(+), 55 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 3e37d63..2232608 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -558,7 +558,8 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,
const char *name)
int result;
- struct pci_slot *pci_slot;
+ struct pci_dev *dev;
+ struct pci_slot *pci_slot, *tmp_slot = NULL;
if (slot == NULL)
return -ENODEV;
@@ -570,9 +571,17 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,
return -EINVAL;
- /* Check if we have already registered a slot with the same name. */
- if (get_slot_from_name(name))
- return -EEXIST;
+ /*
+ * If we find a tmp_slot here, it means that another slot
+ * driver has already created a pci_slot for this device.
+ * We care (below) if the existing slot has a different name from
+ * the new name that this particular hotplug driver is requesting.
+ */
+ dev = pci_get_slot(bus, PCI_DEVFN(slot_nr, 0));
+ if (dev && dev->slot) {
+ tmp_slot = dev->slot;
+ pci_dev_put(dev);
+ }

I have two comments here.

(1) I think the reference counter of the device will be leaked if
(dev == NULL) && (dev->slot != NULL). We need pci_dev_put() whenever
dev is not NULL.

(2) When the slot is empty, the 'dev' will be always NULL. Therefore,
'tmp_slot' will be always NULL on the empty slot here. Because of
this, the following code to rename the slot will not work on the
empty slot, I think.

- * Allow pcihp drivers to override the ACPI_PCI_SLOT name.
+ * Allow pcihp drivers to override existing slot name.
- if (strcmp(kobject_name(&pci_slot->kobj), name)) {
- result = kobject_rename(&pci_slot->kobj, name);
+ if (tmp_slot && strcmp(kobject_name(&tmp_slot->kobj), name)) {
+ result = pci_rename_slot(pci_slot, name);
if (result) {
return result;

Kenji Kaneshige

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at