try_module_get code understanding

try_module_get code understanding

[Shreyansh Jain]

Dear List,

I agree that this issue certainly doesn't require to be in this list (rightful
place being kernewbies) but I tried that and got no response - so trying my luck
here.

I was going through try_module_get function in include/linux/module.h file
(2.6.22 stock kernel) - which is like:

-----
static inline int try_module_get(struct module *module){
  int ret = 1;   <--- error case when !module
    if (module) {
         unsigned int cpu = get_cpu();
         if (likely(module_is_live(module)))
                local_inc(&module->ref[cpu].count);
         else
                ret = 0;   <--- error case
         put_cpu();
  }
  return ret;   <----
}
----

What I understand about the code flow is:
-- module live would return the flag stating that this module can be reference
and is NOT being removed currently.

1. In case the module pointer passed is invalid (NULL) this function would
return 1 (error case)
2. In case the module pointer is OK, and module is currently not being removed,
reference count would be incremented and 1 returned (non error case)
3. In case the module pointer is OK, and module reference count can NOT be
increased, 0 would be returned (error case).

As you can observe from above points, 0 and 1 are returned for error cases. I am
a little confused and wondering if there is something which I am missing in this
code??.

Can anyone help me out with this? Any help would be highly appreciated.

Regards
Shreyansh

Re: try_module_get code understanding

[Heiko Carstens]

> I was going through try_module_get function in include/linux/module.h file
> (2.6.22 stock kernel) - which is like:
> 
> -----
> static inline int try_module_get(struct module *module){
>   int ret = 1;   <--- error case when !module
>     if (module) {
>          unsigned int cpu = get_cpu();
>          if (likely(module_is_live(module)))
>                 local_inc(&module->ref[cpu].count);
>          else
>                 ret = 0;   <--- error case
>          put_cpu();
>   }
>   return ret;   <----
> }
> ----
> 
> What I understand about the code flow is:
> -- module live would return the flag stating that this module can be reference
> and is NOT being removed currently.
> 
> 1. In case the module pointer passed is invalid (NULL) this function would
> return 1 (error case)
> 2. In case the module pointer is OK, and module is currently not being removed,
> reference count would be incremented and 1 returned (non error case)
> 3. In case the module pointer is OK, and module reference count can NOT be
> increased, 0 would be returned (error case).
> 
> As you can observe from above points, 0 and 1 are returned for error cases. I am
> a little confused and wondering if there is something which I am missing in this
> code??.
> 
> Can anyone help me out with this? Any help would be highly appreciated.

Somewhere in module.h you have:

#ifdef MODULE
#define THIS_MODULE (&__this_module)
#else  /* !MODULE */
#define THIS_MODULE ((struct module *)0)
#endif

So this just means, that THIS_MODULE is NULL for compiled in modules
and therefore try_module_get(NULL) succeeds. It's not an error case.

Re: try_module_get code understanding

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

On Thu, 27 Sep 2007 05:19:06 -0000, Shreyansh Jain said:

> -----
> static inline int try_module_get(struct module *module){
>   int ret = 1;   <--- error case when !module
>     if (module) {
>          unsigned int cpu = get_cpu();
>          if (likely(module_is_live(module)))
>                 local_inc(&module->ref[cpu].count);
>          else
>                 ret = 0;   <--- error case
>          put_cpu();
>   }
>   return ret;   <----
> }

> 1. In case the module pointer passed is invalid (NULL) this function would
> return 1 (error case)
> 2. In case the module pointer is OK, and module is currently not being removed,
> reference count would be incremented and 1 returned (non error case)
> 3. In case the module pointer is OK, and module reference count can NOT be
> increased, 0 would be returned (error case).
> 
> As you can observe from above points, 0 and 1 are returned for error cases. I am
> a little confused and wondering if there is something which I am missing in this
> code??.

Go look at the call sites for this function - I bet that most of them, if they
check the return code at all, only check for zero or nonzero, because they only
*care* about the case that returns zero. Since they already know they're not
passing a NULL pointer, they don't worry about that case returning a 1.  So
there's only two realistic returns - either the module is live or it isn't.

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: try_module_get code understanding

[Shreyansh Jain]

Thanks for you reply, please see inline.

Heiko Carstens <heiko.carstens <at> de.ibm.com> writes:
> 
[snip]
> > static inline int try_module_get(struct module *module){
> >   int ret = 1;   <--- error case when !module
> >     if (module) {
> >          unsigned int cpu = get_cpu();
> >          if (likely(module_is_live(module)))
> >                 local_inc(&module->ref[cpu].count);
> >          else
> >                 ret = 0;   <--- error case
> >          put_cpu();
> >   }
> >   return ret;   <----
> > }
> > ----
[snip]
 
> Somewhere in module.h you have:
> 
> #ifdef MODULE
> #define THIS_MODULE (&__this_module)
> #else  /* !MODULE */
> #define THIS_MODULE ((struct module *)0)
> #endif
> 
> So this just means, that THIS_MODULE is NULL for compiled in modules
> and therefore try_module_get(NULL) succeeds. It's not an error case.

Agreed.

Then what is return value if my module tries to 'get' a module which does not
exist (and is a module, not in-built)? . Is it '1' ?
Or am I imagining a hypothetical scenario which would not exist?

Thanks again for your fast response.
Shreyansh

Re: try_module_get code understanding

[Heiko Carstens]

> Then what is return value if my module tries to 'get' a module which does not
> exist (and is a module, not in-built)? . Is it '1' ?
> Or am I imagining a hypothetical scenario which would not exist?

That is not supposed to happen. After a module got unloaded there shouldn't
be any objects around anymore that have an .owner member with an address that
points to the now non-existent module.