Re: [PATCH] modprobe: don't check refcount with remove command

Re: [PATCH] modprobe: don't check refcount with remove command

[Lucas De Marchi]

Hi Johannes,

On Thu, May 2, 2013 at 10:23 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The modprobe.d (5) documentation for the "install" command
> states that you could specify
>
> install fred /sbin/modprobe barney; /sbin/modprobe --ignore-install fred
>
> This makes some sense, but then the loading of "barney" is
> hidden from the user who did only "modprobe fred". Thus,
> it seems it should be possible to be able to unload the
> "fred" module with "modprobe -r fred" by configuring the
> "barney" module to also be removed:
>
> remove fred /sbin/rmmod barney fred
>
> (or similar.)
>
> Make this possible by not checking the refcount when an
> unload command was configured.
>
> Reported-by: David Spinadel <david.spinadel@intel.com>
> ---
>  tools/modprobe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/modprobe.c b/tools/modprobe.c
> index a053efb..6b34658 100644
> --- a/tools/modprobe.c
> +++ b/tools/modprobe.c
> @@ -386,7 +386,7 @@ static int rmmod_do_module(struct kmod_module *mod, bool do_dependencies)
>                         goto error;
>         }
>
> -       if (!ignore_loaded) {
> +       if (!ignore_loaded && !cmd) {
>                 int usage = kmod_module_get_refcnt(mod);
>
>                 if (usage > 0) {
> --

Thanks, patch has been applied.

However.... I'd like to strongly advise not to use install/remove
commands for simple dependencies like this. It's much simpler and
faster both for kmod and for who is configuring to use softdeps. Your
example could be very well replace with the single line below:

softdep fred pre: barney

I think we need a better explanation on man page and even remove this
example from there. Install/remove commands are only there because of
the legacy stuff that would stop working otherwise.

Anyway... thanks for catching this.

Lucas De Marchi

Re: [PATCH] modprobe: don't check refcount with remove command

[Johannes Berg]

Hi Lucas,

> Thanks, patch has been applied.

Thanks.

> However.... I'd like to strongly advise not to use install/remove
> commands for simple dependencies like this. It's much simpler and
> faster both for kmod and for who is configuring to use softdeps. Your
> example could be very well replace with the single line below:
> 
> softdep fred pre: barney

Yes, however, I was just copying the man page example for illustration
purposes.

> I think we need a better explanation on man page and even remove this
> example from there. Install/remove commands are only there because of
> the legacy stuff that would stop working otherwise.

Makes sense. However, the use case we have for this is slightly
different, and not a simple dependency -- we have a bit of a reverse
dependency tree:

 modA1 and modA2 depend on modB

However, modB contains the driver struct, so modB is loaded first
automatically and pulls in either modA1 or modA2 depending on some other
information that can't be encoded in direct dependencies. Then, after
having loaded modB, you can't unload modB any more, and David noticed
that a configuration like

remove modB /sbin/rmmod modA1 modA2 modB

(actually it's a bit more complicated, but for no good reason I think)
no longer worked.

johannes