diff mbox

[2/3] pci: Kill rescan under /sys/.../pci/devices/...

Message ID 4DBF3AEA.6090500@kernel.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Yinghai Lu May 2, 2011, 11:14 p.m. UTC
That is not right. the device is already there, there is no reason to rescan it.
We can not get increase resource for them.

Now We already have rescan for pci_bus.

So remove rescan for all pci devices. less confusing

Finally We remove devices, and rescan bus that there were on before.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 Documentation/ABI/testing/sysfs-bus-pci |   10 ----------
 drivers/pci/pci-sysfs.c                 |   19 -------------------
 2 files changed, 29 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jesse Barnes May 9, 2011, 8:54 p.m. UTC | #1
On Mon, 02 May 2011 16:14:50 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> 
> That is not right. the device is already there, there is no reason to rescan it.
> We can not get increase resource for them.
> 
> Now We already have rescan for pci_bus.
> 
> So remove rescan for all pci devices. less confusing
> 
> Finally We remove devices, and rescan bus that there were on before.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

I'm reluctant to break an existing interface like this, even though
it's only in the "testing" section of the ABI.

Alex, any comments?
Alexander Chiang May 13, 2011, 9:05 a.m. UTC | #2
* Jesse Barnes <jbarnes@virtuousgeek.org>:
> On Mon, 02 May 2011 16:14:50 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
> > 
> > That is not right. the device is already there, there is no reason to rescan it.
> > We can not get increase resource for them.
> > 
> > Now We already have rescan for pci_bus.
> > 
> > So remove rescan for all pci devices. less confusing
> > 
> > Finally We remove devices, and rescan bus that there were on before.
> > 
> > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> I'm reluctant to break an existing interface like this, even though
> it's only in the "testing" section of the ABI.
> 
> Alex, any comments?

Oof, yeah. :-/

I just scanned through the patch series and agree that it makes
more sense to put 'rescan' under the bus, and not on individual
devices.

That said, I'm fairly certain that there are existing tools out
there that are depending on this interface, which has been around
for a while.

I think standard ABI deprecation practices should apply here.

Greg, thoughts?

/ac
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH May 13, 2011, 2:43 p.m. UTC | #3
On Fri, May 13, 2011 at 03:05:08AM -0600, Alex Chiang wrote:
> * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > On Mon, 02 May 2011 16:14:50 -0700
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> > > 
> > > That is not right. the device is already there, there is no reason to rescan it.
> > > We can not get increase resource for them.
> > > 
> > > Now We already have rescan for pci_bus.
> > > 
> > > So remove rescan for all pci devices. less confusing
> > > 
> > > Finally We remove devices, and rescan bus that there were on before.
> > > 
> > > Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> > 
> > I'm reluctant to break an existing interface like this, even though
> > it's only in the "testing" section of the ABI.
> > 
> > Alex, any comments?
> 
> Oof, yeah. :-/
> 
> I just scanned through the patch series and agree that it makes
> more sense to put 'rescan' under the bus, and not on individual
> devices.
> 
> That said, I'm fairly certain that there are existing tools out
> there that are depending on this interface, which has been around
> for a while.
> 
> I think standard ABI deprecation practices should apply here.
> 
> Greg, thoughts?

Ooh, I missed the fact that this series removed a sysfs file, you are
right, we can't do that.

So no, don't delete this file, please fix any tools that might happen to
be using it, and then, in a year or so, we can revisit this to see if it
makes sense to remove it.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Chiang May 13, 2011, 4:04 p.m. UTC | #4
* Greg KH <greg@kroah.com>:
> On Fri, May 13, 2011 at 03:05:08AM -0600, Alex Chiang wrote:
> > * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > > Yinghai Lu <yinghai@kernel.org> wrote:
> > > > 
> > > > So remove rescan for all pci devices. less confusing
> > > 
> > > I'm reluctant to break an existing interface like this, even though
> > > it's only in the "testing" section of the ABI.
> > > 
> > > Alex, any comments?
> > 
> > I think standard ABI deprecation practices should apply here.
> 
> Ooh, I missed the fact that this series removed a sysfs file, you are
> right, we can't do that.
> 
> So no, don't delete this file, please fix any tools that might happen to
> be using it, and then, in a year or so, we can revisit this to see if it
> makes sense to remove it.

I thought we had some "best practices" to do that, like printing
out a big scary deprecation message when the file is accessed,
and documenting it in Documentation/ABI somewhere.

/ac
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg KH May 13, 2011, 6:59 p.m. UTC | #5
On Fri, May 13, 2011 at 10:04:15AM -0600, Alex Chiang wrote:
> * Greg KH <greg@kroah.com>:
> > On Fri, May 13, 2011 at 03:05:08AM -0600, Alex Chiang wrote:
> > > * Jesse Barnes <jbarnes@virtuousgeek.org>:
> > > > Yinghai Lu <yinghai@kernel.org> wrote:
> > > > > 
> > > > > So remove rescan for all pci devices. less confusing
> > > > 
> > > > I'm reluctant to break an existing interface like this, even though
> > > > it's only in the "testing" section of the ABI.
> > > > 
> > > > Alex, any comments?
> > > 
> > > I think standard ABI deprecation practices should apply here.
> > 
> > Ooh, I missed the fact that this series removed a sysfs file, you are
> > right, we can't do that.
> > 
> > So no, don't delete this file, please fix any tools that might happen to
> > be using it, and then, in a year or so, we can revisit this to see if it
> > makes sense to remove it.
> 
> I thought we had some "best practices" to do that, like printing
> out a big scary deprecation message when the file is accessed,
> and documenting it in Documentation/ABI somewhere.

Printing out such a message would be good, that would allow you to catch
the tools that are doing it and change them.

Documenting that the file is going away is for the
Documentation/feature_removal.txt file, not the ABI files.

So that all sounds like a good idea.

Do you know what tools use this file?  Any pointers to them would
probably be appreciated.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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
@@ -272,24 +272,6 @@  struct bus_attribute pci_bus_attrs[] = {
 	__ATTR_NULL
 };
 
-static ssize_t
-dev_rescan_store(struct device *dev, struct device_attribute *attr,
-		 const char *buf, size_t count)
-{
-	unsigned long val;
-	struct pci_dev *pdev = to_pci_dev(dev);
-
-	if (strict_strtoul(buf, 0, &val) < 0)
-		return -EINVAL;
-
-	if (val) {
-		mutex_lock(&pci_remove_rescan_mutex);
-		pci_rescan_bus(pdev->bus);
-		mutex_unlock(&pci_remove_rescan_mutex);
-	}
-	return count;
-}
-
 static void remove_callback(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -363,7 +345,6 @@  struct device_attribute pci_dev_attrs[]
 	__ATTR(msi_bus, 0644, msi_bus_show, msi_bus_store),
 #ifdef CONFIG_HOTPLUG
 	__ATTR(remove, (S_IWUSR|S_IWGRP), NULL, remove_store),
-	__ATTR(rescan, (S_IWUSR|S_IWGRP), NULL, dev_rescan_store),
 #endif
 	__ATTR_NULL,
 };
Index: linux-2.6/Documentation/ABI/testing/sysfs-bus-pci
===================================================================
--- linux-2.6.orig/Documentation/ABI/testing/sysfs-bus-pci
+++ linux-2.6/Documentation/ABI/testing/sysfs-bus-pci
@@ -83,16 +83,6 @@  Description:
 		and re-discover devices removed earlier from this
 		part of the device tree.  Depends on CONFIG_HOTPLUG.
 
-What:		/sys/bus/pci/devices/.../rescan
-Date:		January 2009
-Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
-Description:
-		Writing a non-zero value to this attribute will
-		force a rescan of the device's parent bus and all
-		child buses, and re-discover devices removed earlier
-		from this part of the device tree.
-		Depends on CONFIG_HOTPLUG.
-
 What:		/sys/bus/pci/devices/.../reset
 Date:		July 2009
 Contact:	Michael S. Tsirkin <mst@redhat.com>