PCI: Fix fakephp deadlock

If the fakephp driver is used to emulate removal of a PCI device by
writing text string "0" to the "power" sysfs attribute file, this causes
its parent directory and its contents (including the "power" file) to be
deleted before the write operation returns. Unfortunately, it ends up
in a deadlock waiting for itself to complete.

The deadlock is as follows: sysfs_write_file calls flush_write_buffer
which calls sysfs_get_active_two before calling power_write_file in
pci_hotplug_core.c via the sysfs store operation. The power_write_file
function calls disable_slot in fakephp.c via the slot operation. The
disable_slot function calls remove_slot which calls pci_hp_deregister
(back in pci_hotplug_core.c) which calls fs_remove_slot which calls
sysfs_remove_file to remove the "power" file. The sysfs_remove_file
function calls sysfs_hash_and_remove which calls sysfs_addrm_finish
which calls sysfs_deactivate. The sysfs_deactivate function sees that
something has an active reference on the sysfs_dirent (from the
previous call to sysfs_get_active_two back up the call stack somewhere)
so waits for the active reference to go away, which is of course
impossible.

The problem has been present since 2.6.21.

This patch breaks the deadlock by queuing work queue items on a single-
threaded work queue to remove a slot from sysfs, and to rescan the PCI
buses. There is also some protection against disabling a slot that is
already being removed.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: Kristen Accardi <kristen.c.accardi@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

authored by Ian Abbott and committed by Greg Kroah-Hartman 5c796ae7 4600c9d7

+35 -4
+35 -4
drivers/pci/hotplug/fakephp.c
··· 39 39 #include <linux/init.h> 40 40 #include <linux/string.h> 41 41 #include <linux/slab.h> 42 + #include <linux/workqueue.h> 42 43 #include "../pci.h" 43 44 44 45 #if !defined(MODULE) ··· 64 63 struct list_head node; 65 64 struct hotplug_slot *slot; 66 65 struct pci_dev *dev; 66 + struct work_struct remove_work; 67 + unsigned long removed; 67 68 }; 68 69 69 70 static int debug; 70 71 static LIST_HEAD(slot_list); 72 + static struct workqueue_struct *dummyphp_wq; 73 + 74 + static void pci_rescan_worker(struct work_struct *work); 75 + static DECLARE_WORK(pci_rescan_work, pci_rescan_worker); 71 76 72 77 static int enable_slot (struct hotplug_slot *slot); 73 78 static int disable_slot (struct hotplug_slot *slot); ··· 116 109 slot->name = &dev->dev.bus_id[0]; 117 110 dbg("slot->name = %s\n", slot->name); 118 111 119 - dslot = kmalloc(sizeof(struct dummy_slot), GFP_KERNEL); 112 + dslot = kzalloc(sizeof(struct dummy_slot), GFP_KERNEL); 120 113 if (!dslot) 121 114 goto error_info; 122 115 ··· 169 162 retval = pci_hp_deregister(dslot->slot); 170 163 if (retval) 171 164 err("Problem unregistering a slot %s\n", dslot->slot->name); 165 + } 166 + 167 + /* called from the single-threaded workqueue handler to remove a slot */ 168 + static void remove_slot_worker(struct work_struct *work) 169 + { 170 + struct dummy_slot *dslot = 171 + container_of(work, struct dummy_slot, remove_work); 172 + remove_slot(dslot); 172 173 } 173 174 174 175 /** ··· 282 267 pci_rescan_buses(&pci_root_buses); 283 268 } 284 269 270 + /* called from the single-threaded workqueue handler to rescan all pci buses */ 271 + static void pci_rescan_worker(struct work_struct *work) 272 + { 273 + pci_rescan(); 274 + } 285 275 286 276 static int enable_slot(struct hotplug_slot *hotplug_slot) 287 277 { 288 278 /* mis-use enable_slot for rescanning of the pci bus */ 289 - pci_rescan(); 279 + cancel_work_sync(&pci_rescan_work); 280 + queue_work(dummyphp_wq, &pci_rescan_work); 290 281 return -ENODEV; 291 282 } 292 283 ··· 327 306 err("Can't remove PCI devices with other PCI devices behind it yet.\n"); 328 307 return -ENODEV; 329 308 } 309 + if (test_and_set_bit(0, &dslot->removed)) { 310 + dbg("Slot already scheduled for removal\n"); 311 + return -ENODEV; 312 + } 330 313 /* search for subfunctions and disable them first */ 331 314 if (!(dslot->dev->devfn & 7)) { 332 315 for (func = 1; func < 8; func++) { ··· 353 328 /* remove the device from the pci core */ 354 329 pci_remove_bus_device(dslot->dev); 355 330 356 - /* blow away this sysfs entry and other parts. */ 357 - remove_slot(dslot); 331 + /* queue work item to blow away this sysfs entry and other parts. */ 332 + INIT_WORK(&dslot->remove_work, remove_slot_worker); 333 + queue_work(dummyphp_wq, &dslot->remove_work); 358 334 359 335 return 0; 360 336 } ··· 366 340 struct list_head *next; 367 341 struct dummy_slot *dslot; 368 342 343 + destroy_workqueue(dummyphp_wq); 369 344 list_for_each_safe (tmp, next, &slot_list) { 370 345 dslot = list_entry (tmp, struct dummy_slot, node); 371 346 remove_slot(dslot); ··· 377 350 static int __init dummyphp_init(void) 378 351 { 379 352 info(DRIVER_DESC "\n"); 353 + 354 + dummyphp_wq = create_singlethread_workqueue(MY_NAME); 355 + if (!dummyphp_wq) 356 + return -ENOMEM; 380 357 381 358 return pci_scan_buses(); 382 359 }