modprobe patches

[Denys Vlasenko]

On Thursday 05 March 2009 08:37:45 pm Timo Teräs wrote:
> Denys Vlasenko wrote:
> > I also renamed a few fields and variables here and there, added comments.
> > Please review/test current svn. The // comments are TODOs.
> > 
> > For easy reference new modprobe.c is attached.
> 
> > #define MODULE_FLAG_LOADED		0x0001
> > #define MODULE_FLAG_NEED_DEPS		0x0002
> > //Misnomer? Seems to mean "was seen in modules.dep":
> 
> Not really. It gets set for all the modules we want to load. Those are:
> 1. the names implied on command line
> 2. the aliases defined for command line names
> 
> So for only those entries we actually load and store in memory the
> modules.dep stuff.

Are you saying that name "MODULE_FLAG_EXISTS" is descriptive for
the above? I don't think so. Renaming...


> > struct globals {
> > 	llist_t *db; /* MEs of all modules ever seen (caching for speed) */
> > 	llist_t *probes; /* MEs of module(s) requested on cmdline */
> > 	llist_t *cmdline_mopts; /* strings. module options (from cmdline) */
> > 	int num_deps; /* what is this? */
> 
> Number of module entries that we need to load the dependencies for still.
> E.g. initially set as explained above. For each entry found in modules.dep
> during initial pass, it's decremented.
> 
> This way, after loading modules.dep once, we know if there are still
> "virtual" probe names. If there are, we load and parse the system alias
> file. We generally want to avoid this since it's very large, and alias
> matching uses fnmatch(). This same optimization is in module-init-tools
> too.

I am renaming it to num_unresolved_deps then.


> > 	p = config_open2(CONFIG_DEFAULT_DEPMOD_FILE, xfopen_for_read);
> > //Still true?
> > 	/* Modprobe does not work at all without modprobe.dep,
> > 	 * even if the full module name is given. Returning error here
> > 	 * was making us later confuse user with this message:
> > 	 * "module /full/path/to/existing/file/module.ko not found".
> > 	 * It's better to die immediately, with good message: */
> > //	if (p == NULL)
> > //		bb_perror_msg_and_die("can't open '%s'", CONFIG_DEFAULT_DEPMOD_FILE);
> 
> Ah. Yeah, this actually confliced. Apparently did not realize that
> I added the 'x' for xfopen_for_read. The check is no longer necessary.

Ah... yes I didn't notice that. Removing //commented code...


> > 		m = get_modentry(tokens[0]);
> > 		if (m == NULL)
> > 			continue;
> > //Can we skip it too if it is MODULE_FLAG_LOADED?
> 
> Yes and no. Only if we are loading modules. If we are running with
> '-r' to remove all modules then we obviously need the dependencies
> even if it's loaded.

I am adding this then:

                /* Optimization... */
                if ((m->flags & MODULE_FLAG_LOADED)
                 && !(option_mask32 & MODPROBE_OPT_REMOVE)
                ) {
                        continue;
                }


> > 		/* First argument is module name, rest are parameters */
> > 		add_probe(argv[0]);
> > //TODO: looks like G.cmdline_mopts can contain either NULL or just one string, not more?
> > //optimize it them
> > 		llist_add_to(&G.cmdline_mopts, parse_cmdline_module_options(argv));
> 
> Correct.

Trying to do something about it... Ok, replaced lists
or options with one string, should be more compact.

Hmm, ->aliases member is also misnamed.
It's not "aliases of this module", it's
"real names of this module which is an alias"! Great...

Please find new modprobe.c attached.
Lightly tested.

function                                             old     new   delta
gather_options_str                                     -      75     +75
config_file_action                                   388     413     +25
load_modules_dep                                     175     192     +17
add_probe                                             67      72      +5
modprobe_main                                        494     488      -6
gather_options                                        89       -     -89
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 3/1 up/down: 122/-95)            Total: 27 bytes

--
vda

-------------- next part --------------
A non-text attachment was scrubbed...
Name: modprobe.c
Type: text/x-csrc
Size: 10663 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090306/6ddf8e8d/attachment.c>