lkml.org 
[lkml]   [2012]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC PATCH v1 06/22] PCI: use a global lock to serialize PCI root bridge hotplug operations
    Date
    Currently there's no mechanism to protect the global pci_root_buses list
    from dynamic change at runtime. That means, PCI root bridge hotplug
    operations, which dynamically change the pci_root_buses list, may cause
    invalid memory accesses.

    So introduce a global lock to serialize accesses to the pci_root_buses
    list and serialize PCI host bridge hotplug operations.

    Be careful, never try to acquire this global lock from PCI device drivers,
    that may cause deadlocks.

    Signed-off-by: Jiang Liu <liuj97@gmail.com>
    ---
    drivers/acpi/pci_root.c | 8 +++++++-
    drivers/edac/i7core_edac.c | 16 +++++++---------
    drivers/gpu/drm/drm_fops.c | 6 +++++-
    drivers/pci/host-bridge.c | 19 +++++++++++++++++++
    drivers/pci/hotplug/sgi_hotplug.c | 2 ++
    drivers/pci/pci-sysfs.c | 2 ++
    drivers/pci/probe.c | 5 ++++-
    drivers/pci/search.c | 9 ++++++++-
    include/linux/pci.h | 8 ++++++++
    9 files changed, 62 insertions(+), 13 deletions(-)

    diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
    index 7aff631..6bd0e32 100644
    --- a/drivers/acpi/pci_root.c
    +++ b/drivers/acpi/pci_root.c
    @@ -463,6 +463,8 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
    if (!root)
    return -ENOMEM;

    + pci_host_bridge_hotplug_lock();
    +
    segment = 0;
    status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
    &segment);
    @@ -516,7 +518,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
    * TBD: Need PCI interface for enumeration/configuration of roots.
    */

    - /* TBD: Locking */
    list_add_tail(&root->node, &acpi_pci_roots);

    printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
    @@ -622,11 +623,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
    if (device->wakeup.flags.run_wake)
    device_set_run_wake(root->bus->bridge, true);

    + pci_host_bridge_hotplug_unlock();
    +
    return 0;

    end:
    if (!list_empty(&root->node))
    list_del(&root->node);
    + pci_host_bridge_hotplug_unlock();
    kfree(root);
    return result;
    }
    @@ -643,8 +647,10 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
    {
    struct acpi_pci_root *root = acpi_driver_data(device);

    + pci_host_bridge_hotplug_lock();
    device_set_run_wake(root->bus->bridge, false);
    pci_acpi_remove_bus_pm_notifier(device);
    + pci_host_bridge_hotplug_unlock();

    kfree(root);
    return 0;
    diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
    index d27778f..8e6f177 100644
    --- a/drivers/edac/i7core_edac.c
    +++ b/drivers/edac/i7core_edac.c
    @@ -1196,6 +1196,7 @@ static void __init i7core_xeon_pci_fixup(const struct pci_id_table *table)
    * aren't announced by acpi. So, we need to use a legacy scan probing
    * to detect them
    */
    + pci_host_bridge_hotplug_lock();
    while (table && table->descr) {
    pdev = pci_get_device(PCI_VENDOR_ID_INTEL, table->descr[0].dev_id, NULL);
    if (unlikely(!pdev)) {
    @@ -1205,19 +1206,16 @@ static void __init i7core_xeon_pci_fixup(const struct pci_id_table *table)
    pci_dev_put(pdev);
    table++;
    }
    + pci_host_bridge_hotplug_unlock();
    }

    static unsigned i7core_pci_lastbus(void)
    {
    - int last_bus = 0, bus;
    - struct pci_bus *b = NULL;
    -
    - while ((b = pci_find_next_bus(b)) != NULL) {
    - bus = b->number;
    - debugf0("Found bus %d\n", bus);
    - if (bus > last_bus)
    - last_bus = bus;
    - }
    + int last_bus = 0;
    +
    + for (last_bus = 255; last_bus >= 0; last_bus--)
    + if (pci_find_bus(0, last_bus))
    + break;

    debugf0("Last bus %d\n", last_bus);

    diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
    index 123de28..f559b5b 100644
    --- a/drivers/gpu/drm/drm_fops.c
    +++ b/drivers/gpu/drm/drm_fops.c
    @@ -344,9 +344,13 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
    pci_dev_put(pci_dev);
    }
    if (!dev->hose) {
    - struct pci_bus *b = pci_bus_b(pci_root_buses.next);
    + struct pci_bus *b;
    +
    + pci_host_bridge_hotplug_lock();
    + b = pci_find_next_bus(NULL);
    if (b)
    dev->hose = b->sysdata;
    + pci_host_bridge_hotplug_unlock();
    }
    }
    #endif
    diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
    index a68dc61..28d5557 100644
    --- a/drivers/pci/host-bridge.c
    +++ b/drivers/pci/host-bridge.c
    @@ -94,3 +94,22 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
    res->end = region->end + offset;
    }
    EXPORT_SYMBOL(pcibios_bus_to_resource);
    +
    +static DEFINE_MUTEX(pci_host_bridge_lock);
    +
    +/*
    + * Get the lock to serialize PCI host bridge hotplug operations.
    + * It can't be called from PCI device drivers, otherwise it may cause
    + * deadlocks when removing a host bridge.
    + */
    +void pci_host_bridge_hotplug_lock(void)
    +{
    + mutex_lock(&pci_host_bridge_lock);
    +}
    +EXPORT_SYMBOL(pci_host_bridge_hotplug_lock);
    +
    +void pci_host_bridge_hotplug_unlock(void)
    +{
    + mutex_unlock(&pci_host_bridge_lock);
    +}
    +EXPORT_SYMBOL(pci_host_bridge_hotplug_unlock);
    diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
    index f64ca92..e0e5d13 100644
    --- a/drivers/pci/hotplug/sgi_hotplug.c
    +++ b/drivers/pci/hotplug/sgi_hotplug.c
    @@ -690,6 +690,7 @@ static int __init sn_pci_hotplug_init(void)

    INIT_LIST_HEAD(&sn_hp_list);

    + pci_host_bridge_hotplug_lock();
    while ((pci_bus = pci_find_next_bus(pci_bus))) {
    if (!pci_bus->sysdata)
    continue;
    @@ -709,6 +710,7 @@ static int __init sn_pci_hotplug_init(void)
    break;
    }
    }
    + pci_host_bridge_hotplug_unlock();

    return registered == 1 ? 0 : -ENODEV;
    }
    diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
    index 86c63fe..99fefbe 100644
    --- a/drivers/pci/pci-sysfs.c
    +++ b/drivers/pci/pci-sysfs.c
    @@ -295,10 +295,12 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
    return -EINVAL;

    if (val) {
    + pci_host_bridge_hotplug_lock();
    mutex_lock(&pci_remove_rescan_mutex);
    while ((b = pci_find_next_bus(b)) != NULL)
    pci_rescan_bus(b);
    mutex_unlock(&pci_remove_rescan_mutex);
    + pci_host_bridge_hotplug_unlock();
    }
    return count;
    }
    diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
    index ad77ae5..1f64e8d 100644
    --- a/drivers/pci/probe.c
    +++ b/drivers/pci/probe.c
    @@ -23,7 +23,10 @@ struct resource busn_resource = {
    .flags = IORESOURCE_BUS,
    };

    -/* Ugh. Need to stop exporting this to modules. */
    +/*
    + * Ugh. Need to stop exporting this to modules.
    + * Protected by pci_host_bridge_hotplug_{lock|unlock}().
    + */
    LIST_HEAD(pci_root_buses);
    EXPORT_SYMBOL(pci_root_buses);

    diff --git a/drivers/pci/search.c b/drivers/pci/search.c
    index 993d4a0..f1147a7 100644
    --- a/drivers/pci/search.c
    +++ b/drivers/pci/search.c
    @@ -100,6 +100,13 @@ struct pci_bus * pci_find_bus(int domain, int busnr)
    * initiated by passing %NULL as the @from argument. Otherwise if
    * @from is not %NULL, searches continue from next device on the
    * global list.
    + *
    + * Please don't call this function at rumtime if possible.
    + * It's designed to be called at boot time only because it's unsafe
    + * to PCI root bridge hotplug operations. But some drivers do invoke
    + * it at runtime and it's hard to fix those drivers. In such cases,
    + * use pci_host_bridge_hotplug()_{lock|unlock} to protect the PCI root
    + * bus list, but you need to be really careful to avoid deadlock.
    */
    struct pci_bus *
    pci_find_next_bus(const struct pci_bus *from)
    @@ -115,6 +122,7 @@ pci_find_next_bus(const struct pci_bus *from)
    up_read(&pci_bus_sem);
    return b;
    }
    +EXPORT_SYMBOL(pci_find_next_bus);

    /**
    * pci_get_slot - locate PCI device for a given PCI slot
    @@ -353,7 +361,6 @@ EXPORT_SYMBOL(pci_dev_present);

    /* For boot time work */
    EXPORT_SYMBOL(pci_find_bus);
    -EXPORT_SYMBOL(pci_find_next_bus);
    /* For everyone */
    EXPORT_SYMBOL(pci_get_device);
    EXPORT_SYMBOL(pci_get_subsys);
    diff --git a/include/linux/pci.h b/include/linux/pci.h
    index 21fa79e..e02f130 100644
    --- a/include/linux/pci.h
    +++ b/include/linux/pci.h
    @@ -388,6 +388,8 @@ struct pci_host_bridge {
    void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
    void (*release_fn)(struct pci_host_bridge *),
    void *release_data);
    +void pci_host_bridge_hotplug_lock(void);
    +void pci_host_bridge_hotplug_unlock(void);

    /*
    * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
    @@ -1359,6 +1361,12 @@ static inline void pci_unblock_cfg_access(struct pci_dev *dev)
    static inline struct pci_bus *pci_find_next_bus(const struct pci_bus *from)
    { return NULL; }

    +static inline void pci_host_bridge_hotplug_lock(void)
    +{ }
    +
    +static inline void pci_host_bridge_hotplug_unlock(void)
    +{ }
    +
    static inline struct pci_dev *pci_get_slot(struct pci_bus *bus,
    unsigned int devfn)
    { return NULL; }
    --
    1.7.9.5


    \
     
     \ /
      Last update: 2012-08-07 19:02    [W:4.164 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site