Re: [BUG][2.5]deadlock on cpci hot insert

From: Scott Murray (scottm@somanetworks.com)
Date: Thu Jan 16 2003 - 00:33:05 EST


On Wed, 15 Jan 2003, Rusty Lynch wrote:

> When I hot insert a cpci peripheral board into a ZT5084 chassis
> with a ZT5550 system master board, my ZT5550 locks up. I managed
> to isolate the problem to a deadlock with list_lock

[snip deadlock scenario]

Good catch. Up until now, I've not been building with either pre-empt
or SMP support here, which is why I've never been bitten by this.

> I'm not sure which is the correct way to fix this. Maybe a
> cpci_unlocked_find_slot()?

I'm attaching my preferred fix, which I've only just compile tested
at the moment. It removes cpci_find_slot completely through usage (the
first I believe :) of the data member of pci_visit_dev's wrapped_dev
structure. I'd thought about doing something along these lines in the
past, so it isn't totally out of left field. Removing cpci_find_slot
does change the exported board specific driver API, but it was a bit of
an afterthought on my part to EXPORT it anyways, since none of the board
drivers I've built so far use it.

> hmm... pci_visit_dev is exported, so anybody could call it.

That's actually not a problem, since the used pci_visit structures and
the function pointers they contain are static to cpci_hotplug_pci.c.
The functions in cpci_hotplug_pci.c (cpci_*configure_slot) that do call
pci_visit_dev, while not static, are not EXPORTed.

Scott

PS: Any word on whether my ZT5550 driver patch from last Friday fixes
    your ZT5084 chassis issues?

diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5.58/drivers/hotplug/cpci_hotplug.h linux-2.5.58-dev/drivers/hotplug/cpci_hotplug.h
--- linux-2.5.58/drivers/hotplug/cpci_hotplug.h Thu Jan 16 00:06:58 2003
+++ linux-2.5.58-dev/drivers/hotplug/cpci_hotplug.h Wed Jan 15 23:43:28 2003
@@ -75,7 +75,6 @@
 extern int cpci_hp_unregister_controller(struct cpci_hp_controller *controller);
 extern int cpci_hp_register_bus(struct pci_bus *bus, u8 first, u8 last);
 extern int cpci_hp_unregister_bus(struct pci_bus *bus);
-extern struct slot *cpci_find_slot(struct pci_bus *bus, unsigned int devfn);
 extern int cpci_hp_start(void);
 extern int cpci_hp_stop(void);
 
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5.58/drivers/hotplug/cpci_hotplug_core.c linux-2.5.58-dev/drivers/hotplug/cpci_hotplug_core.c
--- linux-2.5.58/drivers/hotplug/cpci_hotplug_core.c Thu Jan 16 00:06:58 2003
+++ linux-2.5.58-dev/drivers/hotplug/cpci_hotplug_core.c Thu Jan 16 00:00:17 2003
@@ -417,34 +417,6 @@
         return 0;
 }
 
-struct slot *
-cpci_find_slot(struct pci_bus *bus, unsigned int devfn)
-{
- struct slot *slot;
- struct slot *found;
- struct list_head *tmp;
-
- if(!bus) {
- return NULL;
- }
-
- spin_lock(&list_lock);
- if(!slots) {
- spin_unlock(&list_lock);
- return NULL;
- }
- found = NULL;
- list_for_each(tmp, &slot_list) {
- slot = list_entry(tmp, struct slot, slot_list);
- if(slot->bus == bus && slot->devfn == devfn) {
- found = slot;
- break;
- }
- }
- spin_unlock(&list_lock);
- return found;
-}
-
 /* This is the interrupt mode interrupt handler */
 void
 cpci_hp_intr(int irq, void *data, struct pt_regs *regs)
@@ -915,6 +887,5 @@
 EXPORT_SYMBOL_GPL(cpci_hp_unregister_controller);
 EXPORT_SYMBOL_GPL(cpci_hp_register_bus);
 EXPORT_SYMBOL_GPL(cpci_hp_unregister_bus);
-EXPORT_SYMBOL_GPL(cpci_find_slot);
 EXPORT_SYMBOL_GPL(cpci_hp_start);
 EXPORT_SYMBOL_GPL(cpci_hp_stop);
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet linux-2.5.58/drivers/hotplug/cpci_hotplug_pci.c linux-2.5.58-dev/drivers/hotplug/cpci_hotplug_pci.c
--- linux-2.5.58/drivers/hotplug/cpci_hotplug_pci.c Thu Jan 16 00:06:58 2003
+++ linux-2.5.58-dev/drivers/hotplug/cpci_hotplug_pci.c Wed Jan 15 23:42:34 2003
@@ -446,7 +446,7 @@
 }
 
 static int configure_visit_pci_dev(struct pci_dev_wrapped *wrapped_dev,
- struct pci_bus_wrapped *wrapped_bus)
+ struct pci_bus_wrapped *wrapped_bus)
 {
         int rc;
         struct pci_dev *dev = wrapped_dev->dev;
@@ -459,8 +459,8 @@
          * We need to fix up the hotplug representation with the Linux
          * representation.
          */
- slot = cpci_find_slot(dev->bus, dev->devfn);
- if(slot) {
+ if(wrapped_dev->data) {
+ slot = (struct slot*) wrapped_dev->data;
                 slot->dev = dev;
         }
 
@@ -482,7 +482,7 @@
 }
 
 static int unconfigure_visit_pci_dev_phase1(struct pci_dev_wrapped *wrapped_dev,
- struct pci_bus_wrapped *wrapped_bus)
+ struct pci_bus_wrapped *wrapped_bus)
 {
         struct pci_dev *dev = wrapped_dev->dev;
 
@@ -525,8 +525,8 @@
         /*
          * Now remove the hotplug representation.
          */
- slot = cpci_find_slot(dev->bus, dev->devfn);
- if(slot) {
+ if(wrapped_dev->data) {
+ slot = (struct slot*) wrapped_dev->data;
                 slot->dev = NULL;
         } else {
                 dbg("No hotplug representation for %02x:%02x.%x",
@@ -634,6 +634,10 @@
                                 continue;
                         wrapped_dev.dev = dev;
                         wrapped_bus.bus = slot->dev->bus;
+ if(i)
+ wrapped_dev.data = NULL;
+ else
+ wrapped_dev.data = (void*) slot;
                         rc = pci_visit_dev(&configure_functions, &wrapped_dev, &wrapped_bus);
                 }
         }
@@ -666,16 +670,21 @@
                 if(dev) {
                         wrapped_dev.dev = dev;
                         wrapped_bus.bus = dev->bus;
+ if(i)
+ wrapped_dev.data = NULL;
+ else
+ wrapped_dev.data = (void*) slot;
                         dbg("%s - unconfigure phase 1", __FUNCTION__);
                         rc = pci_visit_dev(&unconfigure_functions_phase1,
- &wrapped_dev, &wrapped_bus);
- if(rc) {
+ &wrapped_dev,
+ &wrapped_bus);
+ if(rc)
                                 break;
- }
 
                         dbg("%s - unconfigure phase 2", __FUNCTION__);
                         rc = pci_visit_dev(&unconfigure_functions_phase2,
- &wrapped_dev, &wrapped_bus);
+ &wrapped_dev,
+ &wrapped_bus);
                         if(rc)
                                 break;
                 }

-- 
Scott Murray
SOMA Networks, Inc.
Toronto, Ontario
e-mail: scottm@somanetworks.com

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



This archive was generated by hypermail 2b29 : Thu Jan 23 2003 - 22:00:12 EST