PCI: let the core manage slot names
Subsystems (5)
v6 cover PCI: let the core manage slot names
This is v6 of the series that implements a series of changes that allows the PCI core to manage slot names, rather than individual hotplug drivers.
This version fixes the problem that Kenji-san pointed out of multiple hotplug drivers trying to claim the same slot and rename them as well. It implements his suggestion that pci_create_slot take a struct hotplug_slot * param.
Note that pci_create_slot doesn't actually _do_ anything with the 'hotplug' arg, it merely uses it as a signal that renaming might be required. Arguably, the same thing could have been accomplished with the 'rename' param from v5, but I think passing 'hotplug' makes it more explicit that only hotplug drivers should be setting it (and that only they are allowed to rename detected slots for legacy reasons).
Note again, that pci_hp_register is still the one managing the ->hotplug callback. pci_create_slot does _not_ set the callback.
I don't particularly like the fact that pci_create_slot has to peek and see if the ->hotplug callback is set, but I'm slowly coming to the conclusion that this code is just going to be a little ugly because of all these conflicting requirements.
I've tested by loading pci_slot, fakephp (dup_slots=1), and acpiphp; and verifying that:
- duplicate slot names are avoided
- hotplug drivers can override detection driver names
- multiple hotplug drivers cannot claim the same slot
no matter what order or combination you load or unload the drivers.
Kenji-san, please take a look and let me know what you think. This series is based off of Jesse's latest linux-next branch (99f82734).
Thanks!
/ac
v5 -> v6:
- change 'rename' param to a 'hotplug' param
- add Kenji-san's pci_hp_mutex cleanup to patch series
- pci_create_slot now returns -EBUSY to pci_hp_register
v4 -> v5:
- add 'rename' param to pci_create_slot
- make use of 'rename' param in pci_create_slot
- remove v4 serialization
- remove crap false name collsion code from v2 and v3
- rpaphp uses kstrdup (Thanks to Pekka Enberg for suggestion)
v3 -> v4:
- Do not access hotplug_slot_name() before name initialization
- Serialize pci_hp_register/deregister
v2 -> v3:
- incorporate Willy's code review comments
- fix possible memory leak, pointed out by Rolf Eike Beer
- make false name collision detection work for empty slots
- add 'dup_slots' module_param to fakephp to help debug all this ;)
v1 -> v2:
- fix possible false name collisions
---
Alex Chiang (16):
PCI Hotplug: fakephp: add duplicate slot name debugging
PCI: Hotplug core: remove 'name'
PCI: shcphp: remove 'name' parameter
PCI: SGI Hotplug: stop managing bss_hotplug_slot->name
PCI: rpaphp: kmalloc/kfree slot->name directly
PCI: pciehp: remove 'name' parameter
PCI: ibmphp: stop managing hotplug_slot->name
PCI: fakephp: remove 'name' parameter
PCI: cpqphp: stop managing hotplug_slot->name
PCI: cpci_hotplug: stop managing hotplug_slot->name
PCI: acpiphp: remove 'name' parameter
PCI, PCI Hotplug: introduce slot_name helpers
PCI: prevent duplicate slot names
PCI: update pci_create_slot() to take a 'hotplug' param
PCI: rename pci_update_slot_number to pci_renumber_slot PCI Hotplug core: add 'name' param pci_hp_register interface
Kenji Kaneshige (1):
PCI Hotplug: serialize pci_hp_register and pci_hp_deregister
drivers/acpi/pci_slot.c | 2 drivers/pci/hotplug/acpiphp.h | 9 +- drivers/pci/hotplug/acpiphp_core.c | 32 +++--- drivers/pci/hotplug/cpci_hotplug.h | 6 + drivers/pci/hotplug/cpci_hotplug_core.c | 75 ++++++--------- drivers/pci/hotplug/cpci_hotplug_pci.c | 4 - drivers/pci/hotplug/cpqphp.h | 13 +-- drivers/pci/hotplug/cpqphp_core.c | 43 ++++---- drivers/pci/hotplug/fakephp.c | 26 +++-- drivers/pci/hotplug/ibmphp.h | 5 - drivers/pci/hotplug/ibmphp_ebda.c | 19 +--- drivers/pci/hotplug/pci_hotplug_core.c | 64 ++++-------- drivers/pci/hotplug/pciehp.h | 9 +- drivers/pci/hotplug/pciehp_core.c | 49 +++------ drivers/pci/hotplug/pciehp_ctrl.c | 53 ++++++---- drivers/pci/hotplug/pciehp_hpc.c | 1 drivers/pci/hotplug/rpaphp_slot.c | 10 +- drivers/pci/hotplug/sgi_hotplug.c | 18 +-- drivers/pci/hotplug/shpchp.h | 9 +- drivers/pci/hotplug/shpchp_core.c | 52 ++++------ drivers/pci/hotplug/shpchp_ctrl.c | 48 +++++---- drivers/pci/slot.c | 160 +++++++++++++++++++++++-------- include/linux/pci.h | 10 ++ include/linux/pci_hotplug.h | 11 +- 24 files changed, 382 insertions(+), 346 deletions(-)
PATCH v6 1/3 PCI Hotplug core: add 'name' param pci_hp_register interface
Update pci_hp_register() to take a const char *name parameter.
The motivation for this is to clean up the individual hotplug drivers so that each one does not have to manage its own name. The PCI core should be the place where we manage the name.
We update the interface and all callsites first, in a "no functional change" manner, and clean up the drivers later.
---
drivers/pci/hotplug/acpiphp_core.c | 3 ++- drivers/pci/hotplug/cpci_hotplug_core.c | 3 ++- drivers/pci/hotplug/cpqphp_core.c | 3 ++- drivers/pci/hotplug/fakephp.c | 3 ++- drivers/pci/hotplug/ibmphp_ebda.c | 3 ++- drivers/pci/hotplug/pci_hotplug_core.c | 15 ++++++++------- drivers/pci/hotplug/pciehp_core.c | 3 ++- drivers/pci/hotplug/rpaphp_slot.c | 2 +- drivers/pci/hotplug/sgi_hotplug.c | 3 ++- drivers/pci/hotplug/shpchp_core.c | 3 ++- include/linux/pci_hotplug.h | 3 ++- 11 files changed, 27 insertions(+), 17 deletions(-)
drivers/pci/hotplug/acpiphp_core.c +2 -1
@@ -340,7 +340,8 @@ int acpiphp_register_hotplug_slot(struct acpiphp_slot *acpiphp_slot) retval =pci_hp_register (slot->hotplug_slot, acpiphp_slot->bridge->pci_bus, - acpiphp_slot->device); + acpiphp_slot->device, + slot->name);if (retval == -EBUSY ) goto error_hpslot;if (retval) {
drivers/pci/hotplug/cpci_hotplug_core.c +2 -1
@@ -285,7 +285,8 @@ cpci_hp_register_bus(struct pci_bus *bus, u8 first, u8 last) info->attention_status =cpci_get_attention_status (slot);dbg ("registering slot %s" , slot->hotplug_slot->name); - status =pci_hp_register (slot->hotplug_slot, bus, i); + status =pci_hp_register (slot->hotplug_slot, bus, i, + slot->hotplug_slot->name);if (status) {err ("pci_hp_register failed with error %d" , status); goto error_name;
drivers/pci/hotplug/cpqphp_core.c +2 -1
@@ -436,7 +436,8 @@ static int ctrl_slot_setup(struct controller *ctrl, slot_number); result =pci_hp_register (hotplug_slot, ctrl->pci_dev->subordinate, - slot->device); + slot->device, + hotplug_slot->name);if (result) {err ("pci_hp_register failed with error %d\n" , result); goto error_name;
drivers/pci/hotplug/fakephp.c +2 -1
@@ -126,7 +126,8 @@ static int add_slot(struct pci_dev *dev) slot->release = &dummy_release; slot->private = dslot; - retval =pci_hp_register (slot, dev->bus,PCI_SLOT (dev->devfn)); + retval =pci_hp_register (slot, dev->bus,PCI_SLOT (dev->devfn), + slot->name);if (retval) {err ("pci_hp_register failed with error %d\n" , retval); goto error_dslot;
drivers/pci/hotplug/ibmphp_ebda.c +2 -1
@@ -966,7 +966,8 @@ static int __init ebda_rsrc_controller (void)list_for_each_entry (tmp_slot, &ibmphp_slot_head, ibm_slot_list) {snprintf (tmp_slot->hotplug_slot->name,30 ,"%s" ,create_file_name (tmp_slot));pci_hp_register (tmp_slot->hotplug_slot, -pci_find_bus (0 , tmp_slot->bus), tmp_slot->device); +pci_find_bus (0 , tmp_slot->bus), tmp_slot->device, + tmp_slot->hotplug_slot->name); }print_ebda_hpc ();
drivers/pci/hotplug/pci_hotplug_core.c +8 -7
@@ -547,13 +547,15 @@ out:* @bus: bus this slot is on * @slot: pointer to the &struct hotplug_slot to register * @slot_nr: slot number +* @name: name registered with kobject core * * Registers a hotplug slot with the pci hotplug subsystem, which will allow * userspace interaction to the slot. * * Returns 0 if successful, anything else for an error. */ -int pci_hp_register (struct hotplug_slot *slot,struct pci_bus *bus,int slot_nr) +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;
@@ -569,7 +571,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr) }/* Check if we have already registered a slot with the same name. */ -if (get_slot_from_name (slot->name)) +if (get_slot_from_name (name))return -EEXIST ;/*
@@ -577,7 +579,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)* driver and call it here again. If we've already created the * pci_slot, the interface will simply bump the refcount. */ - pci_slot =pci_create_slot (bus, slot_nr, slot->name); + pci_slot =pci_create_slot (bus, slot_nr, name);if (IS_ERR (pci_slot))return PTR_ERR (pci_slot);
@@ -593,8 +595,8 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr)/* * Allow pcihp drivers to override the ACPI_PCI_SLOT name. */ -if (strcmp (kobject_name (&pci_slot->kobj), slot->name)) { - result =kobject_rename (&pci_slot->kobj, slot->name); +if (strcmp (kobject_name (&pci_slot->kobj), name)) { + result =kobject_rename (&pci_slot->kobj, name);if (result) {pci_destroy_slot (pci_slot);return result;
@@ -607,8 +609,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr) result =fs_add_slot (pci_slot);kobject_uevent (&pci_slot->kobj,KOBJ_ADD ); -dbg ("Added slot %s to the list\n" , slot->name); - +dbg ("Added slot %s to the list\n" , name);return result; }
drivers/pci/hotplug/pciehp_core.c +2 -1
@@ -226,7 +226,8 @@ static int init_slots(struct controller *ctrl) duplicate_name: retval =pci_hp_register (hotplug_slot, ctrl->pci_dev->subordinate, - slot->device); + slot->device, + slot->name);if (retval) {/* * If slot N already exists, we'll try to create
drivers/pci/hotplug/rpaphp_slot.c +1 -1
@@ -137,7 +137,7 @@ int rpaphp_register_slot(struct slot *slot) slotno =PCI_SLOT (PCI_DN (slot->dn->child)->devfn); else slotno =-1 ; - retval =pci_hp_register (php_slot, slot->bus, slotno); + retval =pci_hp_register (php_slot, slot->bus, slotno, slot->name);if (retval) {err ("pci_hp_register failed with error %d\n" , retval);return retval;
drivers/pci/hotplug/sgi_hotplug.c +2 -1
@@ -653,7 +653,8 @@ static int sn_hotplug_slot_register(struct pci_bus *pci_bus) bss_hotplug_slot->ops = &sn_hotplug_slot_ops; bss_hotplug_slot->release = &sn_release_slot; - rc =pci_hp_register (bss_hotplug_slot, pci_bus, device); + rc =pci_hp_register (bss_hotplug_slot, pci_bus, device, + bss_hotplug_slot->name);if (rc) goto register_err;
drivers/pci/hotplug/shpchp_core.c +2 -1
@@ -146,7 +146,8 @@ static int init_slots(struct controller *ctrl) slot->hp_slot, slot->number, ctrl->slot_device_offset); duplicate_name: retval =pci_hp_register (slot->hotplug_slot, - ctrl->pci_dev->subordinate, slot->device); + ctrl->pci_dev->subordinate, slot->device, + hotplug_slot->name);if (retval) {/* * If slot N already exists, we'll try to create
include/linux/pci_hotplug.h +2 -1
@@ -165,7 +165,8 @@ struct hotplug_slot { };#define to_hotplug_slot (n) container_of(n, struct hotplug_slot, kobj) -extern int pci_hp_register (struct hotplug_slot *,struct pci_bus *,int nr); +extern int pci_hp_register (struct hotplug_slot *,struct pci_bus *,int nr, +const char *name);extern int pci_hp_deregister (struct hotplug_slot *slot);extern int __must_check pci_hp_change_slot_info (struct hotplug_slot *slot,struct hotplug_slot_info *info);
PATCH v6 2/3 PCI: rename pci_update_slot_number to pci_renumber_slot
In reply to Alex ChiangThe GPL exported symbol pci_update_slot_number has been renamed to pci_renumber_slot. Some of the safety checks were unnecessary and were removed.
---
drivers/pci/slot.c | 15 +++++---------- include/linux/pci.h | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-)
drivers/pci/slot.c +5 -10
@@ -175,7 +175,7 @@ placeholder:EXPORT_SYMBOL_GPL (pci_create_slot);/** -* pci_update_slot_number - update %struct pci_slot -> number +* pci_renumber_slot - update %struct pci_slot -> number * @slot - %struct pci_slot to update * @slot_nr - new number for slot *
@@ -183,27 +183,22 @@ EXPORT_SYMBOL_GPL(pci_create_slot);* created a placeholder slot in pci_create_slot() by passing a -1 as * slot_nr, to update their %struct pci_slot with the correct @slot_nr. */ - -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);/** * pci_destroy_slot - decrement refcount for physical PCI slot
include/linux/pci.h +1 -1
@@ -510,7 +510,7 @@ struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr,const char *name);void pci_destroy_slot (struct pci_slot *slot); -void pci_update_slot_number (struct pci_slot *slot,int slot_nr); +void pci_renumber_slot (struct pci_slot *slot,int slot_nr);int pci_scan_slot (struct pci_bus *bus,int devfn);struct pci_dev *pci_scan_single_device (struct pci_bus *bus,int devfn);void pci_device_add (struct pci_dev *dev,struct pci_bus *bus);
PATCH v6 3/3 PCI: update pci_create_slot() to take a 'hotplug' param
In reply to Alex ChiangSlot detection drivers can co-exist with hotplug drivers. The names of the detected/claimed slots may be different depending on module load order.
For legacy reasons, we need to allow hotplug drivers to override the slot name if a detection driver is loaded first (and they find the same slots).
Creating and overriding slot names should be an atomic operation, otherwise you get a locking nightmare as various drivers race to call pci_create_slot().
pci_create_slot() is already serialized by grabbing the pci_bus_sem.
We update the API and add a 'hotplug' param, which is:
set if the caller is a hotplug driver
NULL if the caller is a detection driver
pci_create_slot() does not actually use the 'hotplug' parameter in this patch. A later patch will add the logic that uses it.
---
drivers/acpi/pci_slot.c | 2 +- drivers/pci/hotplug/pci_hotplug_core.c | 2 +- drivers/pci/slot.c | 4 +++- include/linux/pci.h | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-)
drivers/acpi/pci_slot.c +1 -1
@@ -150,7 +150,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv) }snprintf (name,sizeof (name),"%u" , (u32 )sun); - pci_slot =pci_create_slot (pci_bus, device, name); + pci_slot =pci_create_slot (pci_bus, device, name,NULL );if (IS_ERR (pci_slot)) {err ("pci_create_slot returned %ld\n" ,PTR_ERR (pci_slot));kfree (slot);
drivers/pci/hotplug/pci_hotplug_core.c +1 -1
@@ -579,7 +579,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,* driver and call it here again. If we've already created the * pci_slot, the interface will simply bump the refcount. */ - pci_slot =pci_create_slot (bus, slot_nr, name); + pci_slot =pci_create_slot (bus, slot_nr, name, slot);if (IS_ERR (pci_slot))return PTR_ERR (pci_slot);
drivers/pci/slot.c +3 -1
@@ -83,6 +83,7 @@ static struct kobj_type pci_slot_ktype = {* @parent: struct pci_bus of parent bridge * @slot_nr: PCI_SLOT(pci_dev->devfn) or -1 for placeholder * @name: user visible string presented in /sys/bus/pci/slots/<name> +* @hotplug: set if caller is hotplug driver, NULL otherwise * * PCI slots have first class attributes such as address, speed, width, * and a &struct pci_slot is used to manage them. This interface will
@@ -111,7 +112,8 @@ static struct kobj_type pci_slot_ktype = {*/ struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr, -const char *name) +const char *name, +struct hotplug_slot *hotplug) {struct pci_dev *dev;struct pci_slot *slot;
include/linux/pci.h +2 -1
@@ -508,7 +508,8 @@ struct pci_bus *pci_create_bus(struct device *parent, int bus,struct pci_bus *pci_add_new_bus (struct pci_bus *parent,struct pci_dev *dev,int busnr);struct pci_slot *pci_create_slot (struct pci_bus *parent,int slot_nr, -const char *name); +const char *name, +struct hotplug_slot *hotplug);void pci_destroy_slot (struct pci_slot *slot);void pci_renumber_slot (struct pci_slot *slot,int slot_nr);int pci_scan_slot (struct pci_bus *bus,int devfn);