[openib-general][PATCH][kdapl]: inc/dec module ref count
Guy German
guyg at voltaire.com
Sun Jul 31 08:08:11 PDT 2005
Hi Muli,
Muli Ben-Yehuda <mailto:mulix at mulix.org> wrote:
> On Thu, Jul 28, 2005 at 10:14:15AM +0300, Guy German wrote:
>
>> [kdapl]: increments the module ref count on ia_open, thus making
>> sure the kdapl_ib module would not be rmmod-ed while in use
>>
>> Signed-off-by: Guy German <guyg at voltaire.com>
>>
>> Index: infiniband/ulp/kdapl/ib/dapl_ia.c
>> ===================================================================
>> --- infiniband/ulp/kdapl/ib/dapl_ia.c (revision 2914)
>> +++ infiniband/ulp/kdapl/ib/dapl_ia.c (working copy)
>> @@ -649,6 +649,7 @@ int dapl_ia_open(const char *name, int a
>> status = 0;
>> *ia_ptr = (struct dat_ia *)ia;
>> *async_evd = (struct dat_evd *)evd;
>> + try_module_get(THIS_MODULE);
>
> This does not look right. First, try_module_get() may fail. Second,
> you're calling it in the context of your module - what happens if
> someone tried to remove it while dapl_ia_open() is running and before
> the call?
Wouldn't it be solved by moving the try_module_get call to the
beginning of the dapl_ia_open function ?
You are right - try_module_get() can fail when the module is not ready
to be entered. should be something like:
+ if (!try_module_get(THIS_MODULE))
+ return -EBUSY;
Guy.
>
> The right place to do module refcounting is in the caller, not in the
> callee. In this case - whereever dapl_ia_open() can be called from.
>
>> bail:
>> if (status != 0)
>> @@ -679,6 +680,7 @@ int dapl_ia_close(struct dat_ia *ia_ptr, else
>> status = -EINVAL;
>>
>> + module_put(THIS_MODULE);
>
> Likewise.
>
> Cheers,
> Muli
More information about the general
mailing list