Hi Muli, Muli Ben-Yehuda <mailto:[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> >> >> 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 _______________________________________________ openib-general mailing list [email protected] http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
