Page MenuHomeFreeBSD

Add support for module_verbose
ClosedPublic

Authored by sjg on Feb 10 2022, 10:32 PM.

Details

Summary

Set module_verbose={0,1,2} to control the printing of information
about loaded modules and kernel:

0 None
1 Pathname and size
2 as for 1 but also twiddle for progress

When the loader is verifying modules we already have a
running indication of progress and module_verbose=0 makes sense.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

sjg requested review of this revision.Feb 10 2022, 10:32 PM

generally I like it. I like the idea of making default verbosity 2, and printing the ELF_VERBOSE stuff at level 3 and ditching that #ifdef.

stand/common/load_elf.c
465–466

I'd be tempted to print the ELF_VERBOSE stuff at level 3 and adjust the documented range.

Might I suggest something like this next to the definition of module_verbose:

enum {
        MODULE_VERBOSE_SILENT,  /* 0 */
        MODULE_VERBOSE_QUIET,   /* 1 */
        MODULE_VERBOSE_DEFAULT, /* 2 */
        MODULE_VERBOSE_FULL,    /* 3 */
};

Or something along those lines. Then, rather than comparing against bare 0 or 2 (or potentially 3, as you mentioned here in Phabricator), you'd compare against the symbol. And of course get rid of MODULE_VERBOSE in favor of MODULE_VERBOSE_DEFAULT.

stand/common/console.c
364

v is a (u_long), while module_verbose is an (int). Therefore, shouldn't this be:

module_verbose = (int) v;
stand/common/load_elf.c
467

It appears the intention is to not twiddle if the verbosity is decreased below the default value module_verbose. In that case, shouldn't this 2 actually be MODULE_VERBOSE?

Might I suggest something like this next to the definition of module_verbose:

enum {
        MODULE_VERBOSE_SILENT,  /* 0 */
        MODULE_VERBOSE_QUIET,   /* 1 */
        MODULE_VERBOSE_DEFAULT, /* 2 */
        MODULE_VERBOSE_FULL,    /* 3 */
};

Or something along those lines. Then, rather than comparing against bare 0 or 2 (or potentially 3, as you mentioned here in Phabricator), you'd compare against the symbol. And of course get rid of MODULE_VERBOSE in favor of MODULE_VERBOSE_DEFAULT.

The point of the #define is to allow build time control of the default, I'm building my loader.efi with -DMODULE_VERBOSE=0

Use MODULE_VERBOSE_TWIDDLE as default

sjg marked 2 inline comments as done.Feb 11 2022, 3:55 PM
sjg added inline comments.
stand/common/load_elf.c
467

No, because again the purpose of the #define is to control the default.

in general, I'd propose that we use >= or > for the test rather than == or != to avoid encoding knowledge about the levels.

stand/common/load_elf.c
466–476

here maybe

469

MODULE_VERBOSE_SILENT

587

here

In D34245#775302, @imp wrote:

in general, I'd propose that we use >= or > for the test rather than == or != to avoid encoding knowledge about the levels.

So we could, for example, have a FULLER :)

Pass correct initial value to env_setenv

This revision is now accepted and ready to land.Feb 13 2022, 5:00 AM
This revision was automatically updated to reflect the committed changes.