Re: Revealing unload_lock to everyone

[Dave Hansen]

Horst von Brand wrote:
 > Dave Hansen <[e-mail]> 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
[e-mail]