Denys Vlasenko wrote:
> 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...
Err... I actually I was talking about MODULE_FLAG_NEED_DEPS.
Apparently you were talking about MODULE_FLAG_EXISTS. And yes,
that's basically "exists in modules.dep". That's way we figure
if we need to load the big aliases file. If module name is not
in modules.dep we do more work to figure out what to do.
Sorry for my confusion.
>>> 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.
Ok.
>>> 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;
> }
Looks ok, if the extra cost of few bytes does not matter.
>>> /* 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.
Ok, will check.
> 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...
Correct. It's what you take the "aliases" to mean. But yes,
saying explicitly that "real_names" and the main entry is the
"virtual alias entry" might be less ambiguous.
Didn't think that way. I guess because module-init-tools
has same naming I took it for granted.
> Please find new modprobe.c attached.
> Lightly tested.
Ok. I'll check it and test it now.
- Timo
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
