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.
> 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.
> while (config_read(p, tokens, 3, 2, "# \t", PARSE_NORMAL)) {
> //Use index_in_strings?
Good idea.
> } else if (!(m2->flags & MODULE_FLAG_LOADED)) {
> options = gather_options(NULL, m2->options);
> if (m == m2)
> options = gather_options(options,
> G.cmdline_mopts);
> //TODO: looks like G.cmdline_mopts can contain either NULL or just one
> string, not more?
Correct.
> static void load_modules_dep(void)
> {
> struct module_entry *m;
> char *colon, *tokens[2];
> parser_t *p;
>
> 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.
> 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.
> /* 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.
Will test tomorrow.
- Timo
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
