Hi James,

That's an interesting point.
I don't know why the ib_mthca can be rmmod-ed at any 
time, without dependencies (I guess it's a trusted env.
issue).
I need to check what happens if you rmmod ib_mthca and 
then calling ia_close. It supposed to decrement 
the ref count even on failure from the hca.

Any way I think that in a production level kernel
you are not supposed to do rmmod AT ALL.

Thanks,
Guy.



-----Original Message-----
From: James Lentini [mailto:[EMAIL PROTECTED]
Sent: Thu 7/28/2005 10:01 PM
To: Guy German
Cc: openib-general
Subject: Re: [openib-general][PATCH][kdapl]: inc/dec module ref count
 

Hmm...this is a tricky one. The ib_mthca module can be rmmod-ed at any 
time. That would break things in kdapl. Consider this scenario:

  consumer opens IA
  rmmod ib_mthca (IB support is pulled out from under us)
  rmmod kdapl_ib (user is trying to fix things)

With this patch, the kdapl_ib module's use count would be 1. The user 
would only be able to rmmod kdapl_ib if the kernel was compiled with 
CONFIG_MODULE_FORCE_UNLOAD.

>From my perspective, the current behavior seems better given the 
situation with ib_mthca. This seems acceptable if the ability to rmmod 
is restricted to knowledgeable sysadmins. What do you think?

One more comment below:

On Thu, 28 Jul 2005, 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);

We've been using the following convention to indicate that we don't 
care about the return value:

  (void) try_module_get(THIS_MODULE);

>
> bail:
>       if (status != 0)
> @@ -679,6 +680,7 @@ int dapl_ia_close(struct dat_ia *ia_ptr,
>       else
>               status = -EINVAL;
>
> +     module_put(THIS_MODULE);
>       return status;
> }
>

_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to