Messages in this thread Patch in this message | | | Date | Wed, 23 Jan 2008 10:42:39 -0800 | From | Greg KH <> | Subject | Re: [RESEND][PATCH-2.6.24-rc8] Fix fakephp deadlock |
| |
On Wed, Jan 23, 2008 at 06:38:22PM +0000, Ian Abbott wrote: > On 23/01/08 17:46, Greg KH wrote: >> On Tue, Jan 22, 2008 at 02:28:08PM +0000, Ian Abbott wrote: >>> #include <linux/init.h> >>> #include <linux/string.h> >>> #include <linux/slab.h> >>> +#include <linux/workqueue.h> >>> #include "../pci.h" >>> #if !defined(MODULE) >>> @@ -63,10 +64,13 @@ struct dummy_slot { >>> struct list_head node; >>> struct hotplug_slot *slot; >>> struct pci_dev *dev; >>> + struct work_struct remove_work; >>> + unsigned long removed; >> You are treating "removed" as an atomic value, so why not just make it >> an atomic_t? > > Because I'm using it as a boolean?
Heh, an unsigned long as a boolean? Come on... :)
>> And what is protecting the fact that the flag could be set right after >> it gets checked? I don't see a lock here :) > > Okay, it looks like there might be a race condition between enable_slot() > and disable_slot() if some other task calls disable_slot() while > enable_slot() is between the test_bit() and flush_workqueue() calls. I can > fix that by avoiding the call to flush_workqueue() in enable_slot() and > allocating and queueing a work queue item to defer the call to > pci_rescan(). And enable_slot() won't then need to check if the slot was > marked as removed - it can just go ahead and allocate and queue a work > item.
That sounds reasonable.
thanks,
greg k-h
| |