[0/12] More Driver Model Locking Changes

[0/12] More Driver Model Locking Changes

[Patrick Mochel]

Here is the next round of driver model locking changes. These build off of
the previous set of changes, including the klist patch. They eradicate all
of the uses of the subsystems' rwsem in the driver core.

It does include the fix posted earlier that happened when removing the
driver.

A summary is listed below. The patches follow.

Thanks,


	Pat


You may pull from

	bk://kernel.bkbits.net:/home/mochel/linux-2.6-core

Which will update the following files:

 drivers/base/bus.c        |   12 -------
 drivers/base/core.c       |   51 ++++++++----------------------
 drivers/base/dd.c         |   77 +++++++++++++++++++++++++---------------------
 drivers/base/driver.c     |    3 -
 drivers/scsi/scsi_sysfs.c |   14 +++++---
 drivers/usb/core/usb.c    |    4 +-
 include/linux/device.h    |   13 +------
 include/linux/klist.h     |    2 +
 lib/klist.c               |   21 +++++++++++-
 9 files changed, 92 insertions(+), 105 deletions(-)

through these ChangeSets:

<mochel@digitalimplant.org> (05/03/24 1.2250)
   [driver core] Fix up bogus comment.
   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2249)
   [driver core] Use a klist for device child lists.

   - Use klist iterator in device_for_each_child(), making it safe to use for
     removing devices.
   - Remove unused list_to_dev() function.
   - Kills all usage of devices_subsys.rwsem.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2248)
   [scsi] Use device_for_each_child() to unregister devices in scsi_remove_target().


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2247)
   [klist] Don't reference NULL klist pointer in klist_remove().


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2246)
   [driver core] Call klist_del() instead of klist_remove().

   - Can't wait on removing the current item in the list (the positive refcount *because*
     we are using it causes it to deadlock).


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2245)
   [driver core] Remove struct device::driver_list.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2244)
   [driver core] Remove struct device::bus_list.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2243)
   [driver core] Fix up bus code and remove use of rwsem.

   - Don't add devices to bus's embedded kset, since it's not used by anyone anymore.
   - Don't need to take the bus rwsem when calling {device,driver}_attach(), since
     those functions use the klists and the klists' spinlocks.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2242)
   [usb] Fix up USB to use klist_node_attached() instead of list_empty() on lists that will go away.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2241)
   [klist] add klist_node_attached() to determine if a node is on a list or not.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2240)
   [driver core] Use bus_for_each_{dev,drv} for driver binding.

   - Now possible, since the lists are locked using the klist lock and not the
     global rwsem.


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

<mochel@digitalimplant.org> (05/03/24 1.2239)
   [driver core] Remove the unused device_find().


   Signed-off-by: Patrick Mochel <mochel@digitalimplant.org>

Re: [0/12] More Driver Model Locking Changes

[Greg KH]

On Thu, Mar 24, 2005 at 09:54:24PM -0800, Patrick Mochel wrote:
> 
> Here is the next round of driver model locking changes. These build off of
> the previous set of changes, including the klist patch. They eradicate all
> of the uses of the subsystems' rwsem in the driver core.
> 
> It does include the fix posted earlier that happened when removing the
> driver.
> 
> A summary is listed below. The patches follow.

Looks great, I've pulled all of these into my tree.

thanks a lot for doing this work.

greg k-h

Re: [0/12] More Driver Model Locking Changes

[Greg KH]

On Fri, Mar 25, 2005 at 11:20:24AM -0800, Greg KH wrote:
> On Thu, Mar 24, 2005 at 09:54:24PM -0800, Patrick Mochel wrote:
> > 
> > Here is the next round of driver model locking changes. These build off of
> > the previous set of changes, including the klist patch. They eradicate all
> > of the uses of the subsystems' rwsem in the driver core.
> > 
> > It does include the fix posted earlier that happened when removing the
> > driver.
> > 
> > A summary is listed below. The patches follow.
> 
> Looks great, I've pulled all of these into my tree.
> 
> thanks a lot for doing this work.

Oops, I needed a fix for the ieee1394 code (attached and applied to my
trees.

But can you take a look at drivers/scsi/scsi_transport_spi.c, line 265?
That is also going to need fixing up somehow.  Gotta love that FIXME
comment...

thanks,

greg k-h


Subject: [ieee1394] Use device_for_each_child() to unregister devices in nodemgr_remove_host_dev()

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff -Nru a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c
--- a/drivers/ieee1394/nodemgr.c	2005-03-25 12:04:35 -08:00
+++ b/drivers/ieee1394/nodemgr.c	2005-03-25 12:04:35 -08:00
@@ -695,14 +695,15 @@
 	put_device(dev);
 }
 
+static int __nodemgr_remove_host_dev(struct device *dev, void *data)
+{
+	nodemgr_remove_ne(container_of(dev, struct node_entry, device));
+	return 0;
+}
 
 static void nodemgr_remove_host_dev(struct device *dev)
 {
-	struct device *ne_dev, *next;
-
-	list_for_each_entry_safe(ne_dev, next, &dev->children, node)
-		nodemgr_remove_ne(container_of(ne_dev, struct node_entry, device));
-
+	device_for_each_child(dev, NULL, __nodemgr_remove_host_dev);
 	sysfs_remove_link(&dev->kobj, "irm_id");
 	sysfs_remove_link(&dev->kobj, "busmgr_id");
 	sysfs_remove_link(&dev->kobj, "host_id");

Re: [0/12] More Driver Model Locking Changes

[Greg KH]

On Fri, Mar 25, 2005 at 03:39:52PM -0800, Greg KH wrote:
> But can you take a look at drivers/scsi/scsi_transport_spi.c, line 265?
> That is also going to need fixing up somehow.  Gotta love that FIXME
> comment...

Ok, the patch below seems to fix it, but I would like some validation I
did the correct thing.

Oh, looks like pci express now has problems too, I'll go hit that one
next...

thanks,

greg k-h

-------------

[scsi] use device_for_each_child() to properly access child devices.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff -Nru a/drivers/scsi/scsi_transport_spi.c b/drivers/scsi/scsi_transport_spi.c
--- a/drivers/scsi/scsi_transport_spi.c	2005-03-25 16:03:09 -08:00
+++ b/drivers/scsi/scsi_transport_spi.c	2005-03-25 16:03:09 -08:00
@@ -254,17 +254,21 @@
 spi_transport_rd_attr(rti, "%d\n");
 spi_transport_rd_attr(pcomp_en, "%d\n");
 
+/* we only care about the first child device so we return 1 */
+static int child_iter(struct device *dev, void *data)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	spi_dv_device(sdev);
+	return 1;
+}
+
 static ssize_t
 store_spi_revalidate(struct class_device *cdev, const char *buf, size_t count)
 {
 	struct scsi_target *starget = transport_class_to_starget(cdev);
 
-	/* FIXME: we're relying on an awful lot of device internals
-	 * here.  We really need a function to get the first available
-	 * child */
-	struct device *dev = container_of(starget->dev.children.next, struct device, node);
-	struct scsi_device *sdev = to_scsi_device(dev);
-	spi_dv_device(sdev);
+	device_for_each_child(&starget->dev, NULL, child_iter);
 	return count;
 }
 static CLASS_DEVICE_ATTR(revalidate, S_IWUSR, NULL, store_spi_revalidate);

Re: [0/12] More Driver Model Locking Changes

[Patrick Mochel]

On Fri, 25 Mar 2005, Greg KH wrote:

> On Fri, Mar 25, 2005 at 03:39:52PM -0800, Greg KH wrote:
> > But can you take a look at drivers/scsi/scsi_transport_spi.c, line 265?
> > That is also going to need fixing up somehow.  Gotta love that FIXME
> > comment...

Heh, funky.

> Ok, the patch below seems to fix it, but I would like some validation I
> did the correct thing.

It looks Ok to me.

> Oh, looks like pci express now has problems too, I'll go hit that one
> next...

Bah, sorry about that. What config are you using to test, 'allmodconfig'?

Thanks,


	Pat

Re: [0/12] More Driver Model Locking Changes

[Greg KH]

On Fri, Mar 25, 2005 at 06:24:58PM -0800, Patrick Mochel wrote:
> On Fri, 25 Mar 2005, Greg KH wrote:
> > Oh, looks like pci express now has problems too, I'll go hit that one
> > next...
> 
> Bah, sorry about that. What config are you using to test, 'allmodconfig'?

Yup.  Looks like pci express is doing some odd stuff, I'll go fix that
up now.

thanks,

greg k-h

Re: [0/12] More Driver Model Locking Changes

[Greg KH]

On Fri, Mar 25, 2005 at 04:03:09PM -0800, Greg KH wrote:
> 
> Oh, looks like pci express now has problems too, I'll go hit that one
> next...

Here's a fix for pci express.  For some reason I don't think they are
using the driver model properly here, but I could be wrong...

thanks,

greg k-h

[pcie] use device_for_each_child() to properly access child devices.
  
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff -Nru a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
--- a/drivers/pci/pcie/portdrv_core.c	2005-03-25 20:44:04 -08:00
+++ b/drivers/pci/pcie/portdrv_core.c	2005-03-25 20:44:04 -08:00
@@ -232,9 +232,6 @@
 	/* Initialize generic device interface */
 	device = &dev->device;
 	memset(device, 0, sizeof(struct device));
-	INIT_LIST_HEAD(&device->node);
-	INIT_LIST_HEAD(&device->children);
-	INIT_LIST_HEAD(&device->bus_list);
 	device->bus = &pcie_port_bus_type;
 	device->driver = NULL;
 	device->driver_data = NULL; 
@@ -317,84 +314,71 @@
 }
 
 #ifdef CONFIG_PM
-int pcie_port_device_suspend(struct pci_dev *dev, u32 state)
+
+static int suspend_iter(struct device *dev, void *data)
 {
-	struct list_head 		*head, *tmp;
-	struct device 			*parent, *child;
-	struct device_driver 		*driver;
 	struct pcie_port_service_driver *service_driver;
+	u32 state = (u32)data;
 
-	parent = &dev->dev;
-	head = &parent->children;
-	tmp = head->next;
-	while (head != tmp) {
-		child = container_of(tmp, struct device, node);
-		tmp = tmp->next;
-		if (child->bus != &pcie_port_bus_type)
-			continue;
-		driver = child->driver;
-		if (!driver)
-			continue;
-		service_driver = to_service_driver(driver);
-		if (service_driver->suspend)  
-			service_driver->suspend(to_pcie_device(child), state);
+	if ((dev->bus == &pcie_port_bus_type) && 
+	    (dev->driver)) {
+		service_driver = to_service_driver(dev->driver);
+		if (service_driver->suspend)
+			service_driver->suspend(to_pcie_device(dev), state);
 	}
+	return 0;
+}
+
+int pcie_port_device_suspend(struct pci_dev *dev, u32 state)
+{
+	device_for_each_child(&dev->dev, (void *)state, suspend_iter);
 	return 0; 
 }
 
-int pcie_port_device_resume(struct pci_dev *dev) 
-{ 
-	struct list_head 		*head, *tmp;
-	struct device 			*parent, *child;
-	struct device_driver 		*driver;
+static int resume_iter(struct device *dev, void *data)
+{
 	struct pcie_port_service_driver *service_driver;
 
-	parent = &dev->dev;
-	head = &parent->children;
-	tmp = head->next;
-	while (head != tmp) {
-		child = container_of(tmp, struct device, node);
-		tmp = tmp->next;
-		if (child->bus != &pcie_port_bus_type)
-			continue;
-		driver = child->driver;
-		if (!driver)
-			continue;
-		service_driver = to_service_driver(driver);
-		if (service_driver->resume)  
-			service_driver->resume(to_pcie_device(child));
+	if ((dev->bus == &pcie_port_bus_type) && 
+	    (dev->driver)) {
+		service_driver = to_service_driver(dev->driver);
+		if (service_driver->resume)
+			service_driver->resume(to_pcie_device(dev));
 	}
-	return 0; 
+	return 0;
+}
 
+int pcie_port_device_resume(struct pci_dev *dev) 
+{ 
+	device_for_each_child(&dev->dev, NULL, resume_iter);
+	return 0; 
 }
 #endif
 
-void pcie_port_device_remove(struct pci_dev *dev)
+static int remove_iter(struct device *dev, void *data)
 {
-	struct list_head 		*head, *tmp;
-	struct device 			*parent, *child;
-	struct device_driver 		*driver;
 	struct pcie_port_service_driver *service_driver;
-	int interrupt_mode = PCIE_PORT_INTx_MODE;
+	int *interrupt_mode = data;
 
-	parent = &dev->dev;
-	head = &parent->children;
-	tmp = head->next;
-	while (head != tmp) {
-		child = container_of(tmp, struct device, node);
-		tmp = tmp->next;
-		if (child->bus != &pcie_port_bus_type)
-			continue;
-		driver = child->driver;
-		if (driver) { 
-			service_driver = to_service_driver(driver);
+	if (dev->bus == &pcie_port_bus_type) {
+		if (dev->driver) {
+			service_driver = to_service_driver(dev->driver);
 			if (service_driver->remove)  
-				service_driver->remove(to_pcie_device(child));
+				service_driver->remove(to_pcie_device(dev));
 		}
-		interrupt_mode = (to_pcie_device(child))->interrupt_mode;
-		put_device(child);
-		device_unregister(child);
+		*interrupt_mode = (to_pcie_device(dev))->interrupt_mode;
+		put_device(dev);
+		device_unregister(dev);
 	}
+	return 0;
+}
+
+void pcie_port_device_remove(struct pci_dev *dev)
+{
+	int interrupt_mode = PCIE_PORT_INTx_MODE;
+
+	device_for_each_child(&dev->dev, &interrupt_mode, remove_iter);
+
 	/* Switch to INTx by default if MSI enabled */
 	if (interrupt_mode == PCIE_PORT_MSIX_MODE)
 		pci_disable_msix(dev);