Re: [Bug 3680] sym53c8xx_2 SMP deadlock on driver load

[Tony Battersby <tonyb@cybernetics.com>]

Willy Tarreau wrote:
> On Wed, Oct 17, 2007 at 10:53:06AM -0400, Tony Battersby wrote:
>   
>>> So we should unconditionally drop the lock (and re-enable
>>> interrupts) and re-acquire it.
>>>       
>> After looking at it carefully, this is true of pci_map_mem, but not
>> pci_unmap_mem.  pci_unmap_mem can be called from both ->detect and
>> ->release.  io_request_lock is held in ->detect but not in ->release.
>> So, your patch locks up the system on module unload.
>>
>> I have put together and tested a new patch which does it correctly,
>> while still trying to make only minimal changes.
>> If you approve, this can go into the next 2.4.x release.
>>     
>
> (...)
>
>   
>> -static void __init pci_unmap_mem(u_long vaddr, u_long size)
>> -{
>> -	if (vaddr)
>> +static void __init pci_unmap_mem(u_long vaddr,
>> +                                 u_long size,
>> +                                 int holding_io_request_lock)
>>     
>
> This is marked __init, and pci_unmap_mem() is called from
> sym_free_resources(), which in turn is called from sym_detach(),
> called from sym53c8xx_release() when unloading module. So the section
> may not be there anymore upon unload. I wonder how this can work right
> now. I'm surely missing something :-/
>
> Willy
>   
In 2.4, include/linux/init.h has the following:

#ifndef MODULE
#define __init          __attribute__ ((__section__ (".text.init")))
#else
#define __init
#endif

So __init has an effect only if it is built-in.  Apparently 2.6 also
discards __init sections in modules after loading, but 2.4 doesn't.  Of
course, it is still a bug.

Do you want me to redo the patch with __init taken out?

Tony