Re: [RFC] Migrating net/sched to new module interface

[Roman Zippel <zippel@linux-m68k.org>]

Hi,

Kronos wrote:

> The  idea  is  to  use   try_module_get()  before  using  any  interface
> registered by the module. Every module has  a state associated to it: if
> the module  has been  loaded but  it is  still in  its init  section its
> state  will be  MODULE_STATE_COMING  (and  try_module_get() will  fail);
> if  the  module is  in  process  of being  unloaded  its  state will  be
> MODULE_STATE_LIVE (and try_module_get() will  fail); otherwise the state
> will be MODULE_STATE_LIVE (and try_module_get() will success).

Rusty had to revert this to the old scheme, as e.g. scsi relied on it.
This means the kernel will oops, if someone gets a reference to a module
and its init function fails.
Above scheme is not without problems either, a failing try_module_get()
suddenly gets a double meaning. Now it also means that the module might
be ready soon, please try again later, what increases the complexity of
the whole module business. 
BTW what makes it currently even more complex is that there is also no
synchronization by the module-init-tools done anymore, a
try_module_get()/request_module()/try_module_get() sequence does give no
definitive answer anymore, whether a module is available or not. The
results will be spurious kmod failures.

> If we use try_module_get() before using the module and module_put() when
> we are done everything will work:
> 
> int create_foo(foo **new) {
>         if (try_module_get(foo->owner) == 0)
>                 /* Module is being unloaded, it's not safe to use it */
>                 return -ENOSYS;
> 
>         *new = foo->new_foo();
> }

It's not that simple, try_module_get() does not everything. It's the
responsibility of the caller to protect the owner pointer, so
try_module_get() itself is only usable within some locking. This means
your patch is still not safe, as the test is done too late.

bye, Roman