lkml.org 
[lkml]   [2008]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 03/15] PCI: prevent duplicate slot names
Alex Chiang wrote:

(snip.)

> diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> index e00266b..b196ea1 100644
> --- a/drivers/pci/hotplug/pci_hotplug_core.c
> +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -572,41 +572,32 @@ 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;
> -
> /*
> - * 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.
> + * Look for existing slot. If we find it, and it was created by a
> + * slot detection driver (ie, doesn't have a ->hotplug()) then we
> + * allow the hotplug driver calling us to rename the slot if desired.
> + *
> + * Otherwise, create the slot and carry on with life.
> */
> - pci_slot = pci_create_slot(bus, slot_nr, name);
> - if (IS_ERR(pci_slot))
> - return PTR_ERR(pci_slot);
> -
> mutex_lock(&pci_hp_mutex);
> - if (pci_slot->hotplug) {
> - mutex_unlock(&pci_hp_mutex);
> - dbg("%s: already claimed\n", __func__);
> - pci_destroy_slot(pci_slot);
> - return -EBUSY;
> + pci_slot = pci_get_pci_slot(bus, slot_nr);
> + if (pci_slot) {
> + if (pci_slot->hotplug) {
> + result = -EBUSY;
> + goto err;
> + }
> +
> + if (strcmp(kobject_name(&pci_slot->kobj), name))
> + if ((result = pci_rename_slot(pci_slot, name)))
> + goto err;
> + } else {
> + pci_slot = pci_create_slot(bus, slot_nr, name);
> + if ((result = IS_ERR(pci_slot)))
> + goto out;
> }
>

I think there still remain race condition. The situation I imagine
is as follows, though I might be misunderstanding something.

If pci_slot driver created pci_slot between pci_get_pci_slot() and
pci_create_slot() in pci_hp_register() in the hotplug driver's thread,
pci_rename_slot() would never be called and name specified by pci_slot
driver would be used instead of the one specified by hotplug driver.


> -void pci_update_slot_number(struct pci_slot *slot, int slot_nr)
> +void pci_renumber_slot(struct pci_slot *slot, int slot_nr)
> {
> - int name_count = 0;
> struct pci_slot *tmp;
>
> down_write(&pci_bus_sem);
>
> list_for_each_entry(tmp, &slot->bus->slots, list) {
> WARN_ON(tmp->number == slot_nr);
> - if (!strcmp(kobject_name(&tmp->kobj), kobject_name(&slot->kobj)))
> - name_count++;
> + goto out;
> }
>
> - if (name_count > 1)
> - printk(KERN_WARNING "pci_update_slot_number found %d slots with the same name: %s\n", name_count, kobject_name(&slot->kobj));
> -
> slot->number = slot_nr;
> +out:
> up_write(&pci_bus_sem);
> }
> -EXPORT_SYMBOL_GPL(pci_update_slot_number);
> +EXPORT_SYMBOL_GPL(pci_renumber_slot);
>

You changed exported symbol name. I think it should be made as
an another patch.

Thanks,
Kenji Kaneshige



\
 
 \ /
  Last update: 2008-10-08 08:05    [W:0.115 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site