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
> prominent.
>
> 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...
> d6a9e9b40be7da84f82eb414c2ad98c5bb69986b
>
> pciehp: Rename duplicate slot name N as N-1, N-2, N-M...
> 167e782e301188c7c7e31e486bbeea5f918324c1
>
> 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);
> + }
>
> /*
> * No problems if we call this interface from both ACPI_PCI_SLOT
> @@ -593,10 +602,10 @@ int pci_hp_register(struct hotplug_slot *slot, struct
> pci_bus *bus, int slot_nr, pci_slot->hotplug = slot;
>
> /*
> - * 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) {
> pci_destroy_slot(pci_slot);
> return result;
> diff --git a/drivers/pci/hotplug/pciehp_core.c
> b/drivers/pci/hotplug/pciehp_core.c index cbd84f8..bed77af 100644
> --- a/drivers/pci/hotplug/pciehp_core.c
> +++ b/drivers/pci/hotplug/pciehp_core.c
> @@ -191,7 +191,6 @@ static int init_slots(struct controller *ctrl)
> struct slot *slot;
> struct hotplug_slot *hotplug_slot;
> struct hotplug_slot_info *info;
> - int len, dup = 1;
> int retval = -ENOMEM;
>
> list_for_each_entry(slot, &ctrl->slot_list, slot_list) {
> @@ -218,24 +217,11 @@ static int init_slots(struct controller *ctrl)
> dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
> "slot_device_offset=%x\n", slot->bus, slot->device,
> slot->hp_slot, slot->number, ctrl->slot_device_offset);
> -duplicate_name:
> retval = pci_hp_register(hotplug_slot,
> ctrl->pci_dev->subordinate,
> slot->device,
> slot->name);
> if (retval) {
> - /*
> - * If slot N already exists, we'll try to create
> - * slot N-1, N-2 ... N-M, until we overflow.
> - */
> - if (retval == -EEXIST) {
> - len = snprintf(slot->name, SLOT_NAME_SIZE,
> - "%d-%d", slot->number, dup++);
> - if (len < SLOT_NAME_SIZE)
> - goto duplicate_name;
> - else
> - err("duplicate slot name overflow\n");
> - }
> err("pci_hp_register failed with error %d\n", retval);
> goto error_info;
> }
> diff --git a/drivers/pci/hotplug/shpchp_core.c
> b/drivers/pci/hotplug/shpchp_core.c index bf50966..cfdd079 100644
> --- a/drivers/pci/hotplug/shpchp_core.c
> +++ b/drivers/pci/hotplug/shpchp_core.c
> @@ -102,7 +102,7 @@ static int init_slots(struct controller *ctrl)
> struct hotplug_slot *hotplug_slot;
> struct hotplug_slot_info *info;
> int retval = -ENOMEM;
> - int i, len, dup = 1;
> + int i;
>
> for (i = 0; i < ctrl->num_slots; i++) {
> slot = kzalloc(sizeof(*slot), GFP_KERNEL);
> @@ -144,23 +144,10 @@ static int init_slots(struct controller *ctrl)
> dbg("Registering bus=%x dev=%x hp_slot=%x sun=%x "
> "slot_device_offset=%x\n", slot->bus, slot->device,
> slot->hp_slot, slot->number, ctrl->slot_device_offset);
> -duplicate_name:
> retval = pci_hp_register(slot->hotplug_slot,
> ctrl->pci_dev->subordinate, slot->device,
> hotplug_slot->name);
> if (retval) {
> - /*
> - * If slot N already exists, we'll try to create
> - * slot N-1, N-2 ... N-M, until we overflow.
> - */
> - if (retval == -EEXIST) {
> - len = snprintf(slot->name, SLOT_NAME_SIZE,
> - "%d-%d", slot->number, dup++);
> - if (len < SLOT_NAME_SIZE)
> - goto duplicate_name;
> - else
> - err("duplicate slot name overflow\n");
> - }
> err("pci_hp_register failed with error %d\n", retval);
> goto error_info;
> }
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 0c6db03..93c55ca 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -78,6 +78,51 @@ static struct kobj_type pci_slot_ktype = {
> .default_attrs = pci_slot_default_attrs,
> };
>
> +static char *make_slot_name(const char *name)
> +{
> + char *new_name;
> + int len, width, dup = 1;
> + struct kobject *dup_slot;
> +
> + new_name = kstrdup(name, GFP_KERNEL);
> + if (!new_name)
> + goto out;
> +
> + /*
> + * Start off allocating enough room for "name-X"
> + */
> + len = strlen(name) + 2;
> + width = 1;
> +
> +try_again:
> + dup_slot = kset_find_obj(pci_slots_kset, new_name);
> + if (!dup_slot)
> + goto out;
> + kobject_put(dup_slot);
> +
> + /*
> + * We hit this the first time through, which gives us
> + * space for terminating NULL, and then every power of 10
> + * afterwards, which gives us space to add another digit
> + * to "name-XX..."
> + */
> + if (dup % width == 0) {
> + len++;
> + width *= 10;
> + }
> +
> + new_name = krealloc(new_name, len, GFP_KERNEL);
> + if (!new_name)
> + goto out;
If krealloc() fails you will leak the old new_name here.
Eike
Attachment:
signature.asc
Description: This is a digitally signed message part.
