Re: [PATCH -v2 1/3] pci: add rescan to /sys/.../pci_bus/.../

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 09, 2011 at 03:34:25PM -0700, Yinghai Lu wrote:
> [PATCH -v2 1/3] pci: add rescan to /sys/.../pci_bus/.../
> 
> After remove the device from /sys, we have to rescan all or
> find out the bridge and access /sys../device/rescan there.
> 
> this patch add /sys/.../pci_bus/.../rescan. So user can rescan more easy.
> that is more clean and easy to understand.
> 
> like after remove 0000:c4:00.0, you can rescan 0000:c4 directly.
> 
> -v2: According to Jesse, use function instead of exposing attr, so could hide
> 	#ifdef in header file.
>      also add code to remove rescan file in remove path.
> 
> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
> 
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |    9 +++++++++
>  drivers/pci/bus.c                       |    4 ++++
>  drivers/pci/pci-sysfs.c                 |   31 +++++++++++++++++++++++++++++++
>  drivers/pci/pci.h                       |   10 ++++++++++
>  drivers/pci/probe.c                     |    5 +++++
>  drivers/pci/remove.c                    |    1 +
>  6 files changed, 60 insertions(+)
> 
> Index: linux-2.6/drivers/pci/pci-sysfs.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-sysfs.c
> +++ linux-2.6/drivers/pci/pci-sysfs.c
> @@ -318,6 +318,37 @@ remove_store(struct device *dev, struct
>  		count = ret;
>  	return count;
>  }
> +
> +static ssize_t
> +dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
> +		 const char *buf, size_t count)
> +{
> +	unsigned long val;
> +	struct pci_bus *bus = to_pci_bus(dev);
> +
> +	if (strict_strtoul(buf, 0, &val) < 0)
> +		return -EINVAL;
> +
> +	if (val) {
> +		mutex_lock(&pci_remove_rescan_mutex);
> +		pci_rescan_bus(bus);
> +		mutex_unlock(&pci_remove_rescan_mutex);
> +	}
> +	return count;
> +}
> +
> +DEVICE_ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_bus_rescan_store);
> +
> +int pci_bus_create_rescan_file(struct pci_bus *bus)
> +{
> +	return device_create_file(&bus->dev, &dev_attr_rescan);
> +}
> +
> +void pci_bus_remove_rescan_file(struct pci_bus *bus)
> +{
> +	device_remove_file(&bus->dev, &dev_attr_rescan);
> +}

No, please make it part of the attribute for this device type, otherwise
you will race with userspace trying to create the file after it has been
announced that the device is present.


> +
>  #endif
>  
>  struct device_attribute pci_dev_attrs[] = {
> Index: linux-2.6/drivers/pci/bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/bus.c
> +++ linux-2.6/drivers/pci/bus.c
> @@ -163,6 +163,10 @@ int pci_bus_add_child(struct pci_bus *bu
>  
>  	bus->is_added = 1;
>  
> +	retval = pci_bus_create_rescan_file(bus);
> +	if (retval)
> +		return retval;
> +

See, you just raced.

>  	retval = device_create_file(&bus->dev, &dev_attr_cpuaffinity);
>  	if (retval)
>  		return retval;

Look this file, and the one after it are also incorrect, don't continue
to add broken code to the kernel please.

thanks,

greg k-h
--
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


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux