[RFC/PATCH] Add pci_walk_bus function to PCI core

[RFC/PATCH] Add pci_walk_bus function to PCI core

[Paul Mackerras]

Greg,

Any comments on this patch?  Would you be amenable to it going in post
2.6.13?

The PCI error recovery infrastructure needs to be able to contact all
the drivers affected by a PCI error event, which may mean traversing
all the devices under a given PCI-PCI bridge.  This patch adds a
function to the PCI core that traverses all the PCI devices on a PCI
bus and under any PCI-PCI bridges on that bus (recursively), calling a
given function for each device.  This provides a way for the error
recovery code to iterate through all devices that are affected by an
error event.  This function was originally written by Linas Vepstas
and moved to drivers/pci/bus.c (and slightly modified) by me.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
diff -urN linux-2.6/drivers/pci/bus.c test-pseries/drivers/pci/bus.c
--- linux-2.6/drivers/pci/bus.c	2005-08-03 10:51:36.000000000 +1000
+++ test-pseries/drivers/pci/bus.c	2005-08-09 17:05:16.000000000 +1000
@@ -150,6 +150,36 @@
 	}
 }
 
+/** pci_walk_bus - walk devices on/under bus, calling callback.
+ *  @top      bus whose devices should be walked
+ *  @cb       callback to be called for each device found
+ *  @userdata arbitrary pointer to be passed to callback.
+ *
+ *  Walk the given bus, including any bridged devices
+ *  on buses under this bus.  Call the provided callback
+ *  on each device found.
+ */
+void pci_walk_bus(struct pci_bus *top, pci_buswalk_cb cb, void *userdata)
+{
+	struct pci_dev *dev, *tmp;
+
+	spin_lock(&pci_bus_lock);
+	list_for_each_entry_safe (dev, tmp, &top->devices, bus_list) {
+		pci_dev_get(dev);
+		spin_unlock(&pci_bus_lock);
+
+		/* Run device routines with the bus unlocked */
+		cb(dev, userdata);
+		if (dev->subordinate)
+			pci_walk_bus(dev->subordinate, cb, userdata);
+
+		spin_lock(&pci_bus_lock);
+		pci_dev_put(dev);
+	}
+	spin_unlock(&pci_bus_lock);
+}
+EXPORT_SYMBOL_GPL(pci_walk_bus);
+
 EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL_GPL(pci_bus_add_device);
 EXPORT_SYMBOL(pci_bus_add_devices);
diff -urN linux-2.6/include/linux/pci.h test-pseries/include/linux/pci.h
--- linux-2.6/include/linux/pci.h	2005-08-10 10:53:31.000000000 +1000
+++ test-pseries/include/linux/pci.h	2005-08-10 11:25:40.000000000 +1000
@@ -864,6 +864,9 @@
 const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, struct pci_dev *dev);
 int pci_scan_bridge(struct pci_bus *bus, struct pci_dev * dev, int max, int pass);
 
+typedef void (*pci_buswalk_cb)(struct pci_dev *, void *);
+void pci_walk_bus(struct pci_bus *top, pci_buswalk_cb cb, void *userdata);
+
 /* kmem_cache style wrapper around pci_alloc_consistent() */
 
 #include <linux/dmapool.h>

Re: [RFC/PATCH] Add pci_walk_bus function to PCI core

[Arjan van de Ven]

On Wed, 2005-08-10 at 11:36 +1000, Paul Mackerras wrote:
> Greg,
> 
> Any comments on this patch?  Would you be amenable to it going in post
> 2.6.13?
> 
> The PCI error recovery infrastructure needs to be able to contact all
> the drivers affected by a PCI error event, which may mean traversing
> all the devices under a given PCI-PCI bridge.  This patch adds a
> function to the PCI core that traverses all the PCI devices on a PCI
> bus and under any PCI-PCI bridges on that bus (recursively), calling a
> given function for each device. 

is there a way to avoid the recursion somehow? Recursion is "not fun"
stack usage wise, esp if you have really deep hierarchies....

Re: [RFC/PATCH] Add pci_walk_bus function to PCI core

[Kyle Moffett]

On Aug 10, 2005, at 02:10:49, Arjan van de Ven wrote:
> On Wed, 2005-08-10 at 11:36 +1000, Paul Mackerras wrote:
>
>> Greg,
>>
>> Any comments on this patch?  Would you be amenable to it going in  
>> post
>> 2.6.13?
>>
>> The PCI error recovery infrastructure needs to be able to contact all
>> the drivers affected by a PCI error event, which may mean traversing
>> all the devices under a given PCI-PCI bridge.  This patch adds a
>> function to the PCI core that traverses all the PCI devices on a PCI
>> bus and under any PCI-PCI bridges on that bus (recursively),  
>> calling a
>> given function for each device.
>
> is there a way to avoid the recursion somehow? Recursion is "not fun"
> stack usage wise, esp if you have really deep hierarchies....

Hmm, it looks like PCI error recovery wants breadth-first recursion, so
you should be able to do some sort of tail-recursion or something.  If
only one error-recovery action on a given subtree can be going at a  
time,
you should be able to add an "error_recovery" linked-list to the device
structure and do something like this:

void recover(...) {
     struct list_head recovery_list = LIST_HEAD_INIT(recovery_list);
     list_add(&dev->error_recovery, &recovery_list);

     while(!list_empty(&recovery_list)) {
         struct some_device_type *dev =
             list_entry(recovery_list->next, struct some_device_type,  
error_recovery);

         dev->some_recovery_function(dev, [...]);

         list_del(&dev->error_recovery);
     }
}

Then each PCI-PCI bridge's some_recovery_function could do this:

void some_recovery_function(struct some_device_type *dev, [...]) {
     struct some_device_type *child;

     actually_do_my_recovery();

     list_for_each_entry(child, dev->some_pci_subdev_list,  
some_pci_list) {
         if (needs_recovery(child))
             list_add_tail(&child->error_recovery,&dev->error_recovery);
     }
}

With such an arrangement, the callstack is as shallow as possible:

recover
     some_recovery_function
         actually_do_my_recovery
         needs_recovery
     childs_recovery_function
     [...]

If you can have multiple simultaneous error-recovery actions per  
subtree,
that wouldn't properly work unless they were exclusive-blocking, IE:
an error recovery action triggers an error on a subtree which must
recover itself.  In that case, with some extra state saved in the  
recover
function and passed to the "some_recovery_function", you could allow the
other recovery to continue before resuming.

If you can have two CPUs recovering the same device tree, I'd be  
inclined
to wonder what kind of strange errors you're causing on the PCI bus :-D,
and I'd be interested in an example of how that could work in any  
sane way.

Cheers,
Kyle Moffett

--
Premature optimization is the root of all evil in programming
   -- C.A.R. Hoare

Re: [RFC/PATCH] Add pci_walk_bus function to PCI core

[Paul Mackerras]

Arjan van de Ven writes:

> is there a way to avoid the recursion somehow? Recursion is "not fun"
> stack usage wise, esp if you have really deep hierarchies....

Yes, since we have pointers up the tree as well as down, it should in
fact be easy.  I'll hack something up.

Paul.

Re: [RFC/PATCH] Add pci_walk_bus function to PCI core

[Greg KH]

On Wed, Aug 10, 2005 at 11:36:58AM +1000, Paul Mackerras wrote:
> Greg,
> 
> Any comments on this patch?  Would you be amenable to it going in post
> 2.6.13?

Looks fine to me.  I'll hold off on applying as you said you were going
to try to get a version without recursion.

thanks,

greg k-h