Alex Chiang wrote:
* Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx>:
Thank you for patches, Alex-san!
I've reviewed those patches and tested them on my ia64 machine
that have both shpc and pcie hotplug slots. Your patch looks
good.
Thank you for reviewing and testing.
As you mentioned, we are considering the problem also from the
view point of slot detection. But I think your patch is needed
regardless of that because there might be platforms whose slots
are detected properly but firmware assigns the physical slot
number wrongly. I think Alex's patch should go to mainline.
That is a good point.
P.S.: I found a possible improvement, though it is not a big
problem and we don't not need to fix it soon. I'd like to tell
you about it just in case. Current pci_hp_register() checks if
name is duplicated first, before checking if another hotplug
driver is already registered to the slot. So, if shpchp/pciehp
driver tries to register hotplug slot that is already registered
by the other hotplug driver (e.g. acpiphp) with the same name,
shpchp/pciehp driver will do as follows:
(1) shpchp/pciehp call pci_hp_register()
(2) pci_hp_register() returns -EEXIST
(3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
(4) pci_hp_register() returns -EBUSY
if pci_hp_register() checked if another hotplug driver is already
registered first, step (2) and (3) could be removed.
Thanks, that seems pretty easy to do.
Would you mind testing this patch as well? You should probably
apply it on top of the other two patches to see how all three
patches interact.
Thanks!
/ac
From: Alex Chiang <achiang@xxxxxxxxxxx>
Subject: [PATCH] PCI hotplug: check for claimed slot before duplicate named slot
Kenji Kaneshige observes that:
If shpchp/pciehp driver tries to register hotplug slot that is
already registered by the other hotplug driver (e.g. acpiphp) with
the same name, shpchp/pciehp driver will do as follows:
(1) shpchp/pciehp call pci_hp_register()
(2) pci_hp_register() returns -EEXIST
(3) shpchp/pciehp call pci_hp_register() with other name ("M-1")
(4) pci_hp_register() returns -EBUSY
If pci_hp_register() checked if another hotplug driver is already
registered first, step (2) and (3) could be removed.
This patch does not prevent the *same* driver from attempting
to register multiple slots with the same name (on systems with
broken firmware). For that situation, we still need to detect
a name collision and return -EEXIST if so.
Signed-off-by: Alex Chiang <achiang@xxxxxx>
---
drivers/pci/hotplug/pci_hotplug_core.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 5f85b1b..9c379b6 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -568,10 +568,6 @@ 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(slot->name))
- return -EEXIST;
-
/*
* No problems if we call this interface from both ACPI_PCI_SLOT
* driver and call it here again. If we've already created the
@@ -587,6 +583,12 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
return -EBUSY;
}
+ /* Check if we have already registered a slot with the same name. */
+ if (get_slot_from_name(slot->name)) {
+ pci_destroy_slot(pci_slot);
+ return -EEXIST;
+ }
+
slot->pci_slot = pci_slot;
pci_slot->hotplug = slot;
@@ -609,7 +611,6 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)
kobject_uevent(&pci_slot->kobj, KOBJ_ADD);
dbg("Added slot %s to the list\n", slot->name);
-
return result;
}
Unfortunately, we can't simply move the following check after pci_create_slot().
- /* Check if we have already registered a slot with the same name. */
- if (get_slot_from_name(slot->name))
- return -EEXIST;
-
With this change, kobject_init_and_add() called in pci_create_slot() will
show stack trace if a hotplug driver attempts to register multiple slot with
the same name. That is, stack trace will be shown on the platform that wrongly
assing the physical slot number to multiple slots. I'm very sorry, but I don't
have enough time to consider how to fix it today.
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
