Revealing unload_lock to everyone

Revealing unload_lock to everyone

[Dave Hansen]

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?

-- 
Dave Hansen
haveblue@us.ibm.com

Re: Revealing unload_lock to everyone

[Keith Owens]

On Thu, 31 Jan 2002 15:58:17 -0800, 
Dave Hansen <haveblue@us.ibm.com> wrote:
>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?

try_inc_mod_count().

Re: Revealing unload_lock to everyone

[Dave Hansen]

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