[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 1/2] rmmod (madprobe-small): don't look for default
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2011-06-30 0:29:32
Message-ID: 201106300229.32227.vda.linux () googlemail ! com
[Download RAW message or body]

On Wednesday 29 June 2011 17:41, Federico Vaga wrote:
> Modules with rmmod are removed by name and it is not important if a module is in
> the default module's path or if the path exist, so it useless check if default
> path exist and require its existence.
> 
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
> ---
>  modutils/modprobe-small.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
> index f5b283b..57b83c0 100644
> --- a/modutils/modprobe-small.c
> +++ b/modutils/modprobe-small.c
> @@ -763,7 +763,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
>  	/* Prevent ugly corner cases with no modules at all */
>  	modinfo = xzalloc(sizeof(modinfo[0]));
>  
> -	if ('i' != applet0) { /* not insmod */
> +	if ('i' != applet0 && 'r' != applet0) { /* not insmod and not rmmod */
>  		/* Goto modules directory */
>  		xchdir(CONFIG_DEFAULT_MODULES_DIR);
>  	}
> @@ -813,7 +813,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
>  		option_mask32 |= OPT_r;
>  	}
>  
> -	if ('i' != applet0) { /* not insmod */
> +	if ('i' != applet0 && 'r' != applet0) { /* not insmod and not rmmod*/
>  		/* Goto $VERSION directory */
>  		xchdir(uts.release);
>  	}

Unfortunately, you can't do this this simply.
modprobe-small treats rmmod as modprobe -r:

        /* are we rmmod? -> simulate modprobe -r */
        if ('r' == applet0) {
                option_mask32 |= OPT_r;
        }

and modprobe -r is not really the same as rmmod,
it will try to unload modules the removed module depends on:

        if (!module_count) {
WHOOPS!
THIS REQUIRES
US TO BE IN PROPER DIR!
                /* Scan module directory. This is done only once.
                 * It will attempt module load, and will exit(EXIT_SUCCESS)
                 * on success. */
...
        } else {
                info = find_alias(name);
        }
        /* rmmod? unload it by name */
        if (is_rmmod) {
                if (delete_module(name, O_NONBLOCK | O_EXCL) != 0) {
                        if (!(option_mask32 & OPT_q))
                                bb_perror_msg("remove '%s'", name);
                        goto ret;
                }
THIS COMMENT ==>
                /* N.B. we do not stop here -
                 * continue to unload modules on which the module depends:
                 * "-r --remove: option causes modprobe to remove a module.
                 * If the modules it depends on are also unused, modprobe
                 * will try to remove them, too." */
        }
        if (!info) {
                /* both dirscan and find_alias found nothing */
                if (!is_rmmod && applet_name[0] != 'd') /* it wasn't rmmod or depmod */
                        bb_error_msg("module '%s' not found", name);
                goto ret;
        }
        /* Iterate thru dependencies, trying to (un)load them */
	...

With your patch, the code will still try to load dependencies
(see where recursive_action is called), and since you didn't
chdir into proper directory, the loading will try to read stuff
in the wrong directory!

Can you add code which avoids dependency loading, and
doesn't try to unload other modules a-la modprobe -r?

-- 
vda
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]