Re: Revealing unload_lock to everyone

Dave Hansen
2002-02-01T15:35:43Z

Horst von Brand wrote:
> Dave Hansen <haveblue@us.ibm.com> said:
>
>>This came up in a conversation about ieee1394_core.c. In 2.5.3, the BKL
>>is used to protect a module from being unloaded. The code looks like
this:
>>
>> lock_kernel();
>> read_lock(&ieee1394_chardevs_lock);
>> file_ops = ieee1394_chardevs[blocknum].file_ops;
>> module = ieee1394_chardevs[blocknum].module;
>> read_unlock(&ieee1394_chardevs_lock);
>> ...
>> INCREF(module);
>> unlock_kernel();
>>
>>
>>The question is, how can we keep the module from being unloaded between
>>the file_ops assignment, and the INCREF. Do we have a general purpose
>>way, other than the BKL, to keep a module from being unloaded? There is
>>unload_lock, but it is static to module.c. We can always make it
>>global, but is there a better solution?
>>
>
> Move the INCREF() up?
>

This is really perverse, but here is why that doesn't work:

module not loaded
INCREF(module); /* this fails, no module loaded*/
interrupt, blah, blah, blah
now module is loaded by insmod or something
module = ieee1394_chardevs[blocknum].module;
module now set, but no refcnt bump has been done because it's newly loaded.
module removed
try to set something which went with the module
*BAM*

So, instead, we used try_mod_inc_count() instead of the local INCREF()
#define and return failure if try_mod_inc_count() fails. Thanks to
Keith Ownens for pointing me to try_mod_inc_count().

--
Dave Hansen
haveblue@us.ibm.com