Re: [PATCH v4 02/15] PCI Hotplug: serialize pci_hp_register/deregister

[Kenji Kaneshige]

I think we no longer need pci_hotplug_slot_list_lock with your patch.

Thanks,
Kenji Kaneshige


Alex Chiang wrote:
> Add explicit locking to pci_hp_register/deregister. For a given pci_slot,
> we want to make sure that the pci_slot->hotplug pointer isn't being
> changed from underneath us.
>
> 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 |    7 +++++++
>  drivers/pci/slot.c                     |    4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
> index 0e7a511..e00266b 100644
> --- a/drivers/pci/hotplug/pci_hotplug_core.c
> +++ b/drivers/pci/hotplug/pci_hotplug_core.c
> @@ -37,6 +37,7 @@
>  #include <linux/init.h>
>  #include <linux/mount.h>
>  #include <linux/namei.h>
> +#include <linux/mutex.h>
>  #include <linux/pci.h>
>  #include <linux/pci_hotplug.h>
>  #include <asm/uaccess.h>
> @@ -554,6 +555,7 @@ out:
>   *
>   * Returns 0 if successful, anything else for an error.
>   */
> +static DEFINE_MUTEX(pci_hp_mutex);
>  int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,
>  			const char *name)
>  {
> @@ -583,7 +585,9 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,
>  	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;
> @@ -591,6 +595,7 @@ int pci_hp_register(struct hotplug_slot *slot, struct pci_bus *bus, int slot_nr,
>  
>  	slot->pci_slot = pci_slot;
>  	pci_slot->hotplug = slot;
> +	mutex_unlock(&pci_hp_mutex);
>  
>  	/*
>  	 * Allow pcihp drivers to override the ACPI_PCI_SLOT name.
> @@ -635,6 +640,7 @@ int pci_hp_deregister(struct hotplug_slot *hotplug)
>  	if (temp != hotplug)
>  		return -ENODEV;
>  
> +	mutex_lock(&pci_hp_mutex);
>  	spin_lock(&pci_hotplug_slot_list_lock);
>  	list_del(&hotplug->slot_list);
>  	spin_unlock(&pci_hotplug_slot_list_lock);
> @@ -645,6 +651,7 @@ int pci_hp_deregister(struct hotplug_slot *hotplug)
>  
>  	hotplug->release(hotplug);
>  	slot->hotplug = NULL;
> +	mutex_unlock(&pci_hp_mutex);
>  	pci_destroy_slot(slot);
>  
>  	return 0;
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index 0c6db03..1fffb27 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -164,10 +164,10 @@ placeholder:
>  	pr_debug("%s: created pci_slot on %04x:%02x:%02x\n",
>  		 __func__, pci_domain_nr(parent), parent->number, slot_nr);
>  
> - out:
> +out:
>  	up_write(&pci_bus_sem);
>  	return slot;
> - err:
> +err:
>  	kfree(slot);
>  	slot = ERR_PTR(err);
>  	goto out;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/