lkml.org 
[lkml]   [2008]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 02/13] PCI: prevent duplicate slot names
    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@virtuousgeek.org
    > Cc: kristen.c.accardi@intel.com
    > Cc: matthew@wil.cx
    > Cc: kaneshige.kenji@jp.fujitsu.com
    > Signed-off-by: Alex Chiang <achiang@hp.com>
    > ---
    >
    > 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) {
    > pci_destroy_slot(pci_slot);
    > return result;

    Thanks,
    Kenji Kaneshige



    \
     
     \ /
      Last update: 2008-09-11 04:51    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean