On Wed, Oct 08, 2008 at 02:47:00PM +0100, Mel Gorman wrote:
> On (08/10/08 13:52), Andy Whitcroft didst pronounce:
> > Add control over verbosity and debugging both in the hugectl and in the
> > library when invoked. Allows the level to be set directly via
> > --verbose <level> and may be incremented via -v. Where the level is set
> > to 99 debugging is also enabled.
> >
>
> I don't think it even needs to go to 99. If it's 1 more than the largest
> sensible verbosity level, I think it'd be reasonable to enable
> debugging. I don't feel strongly though and we can document it either
> way and I'm happy with this behaviour to start with.
I picked that because of the way the README recommends debugging be
enabled:
* Set HUGETLB_VERBOSE=99 and HUGETLB_DEBUG=yes. These options increase
the verbosity of the library and enable extra checking to help diagnose
the problem.
There was no other basis for this choice. The separation of debugging
from verbosity seems to be about adding extra runtime. Verbosity tells
you more, debugging makes more happen. So perhaps it makes some sense
for them to be significantly separate. We can have a separate
--debug/-d if that is preferred.
> > Signed-off-by: Andy Whitcroft <[EMAIL PROTECTED]>
> > ---
> > hugectl.c | 34 +++++++++++++++++++++++++++++++++-
> > 1 files changed, 33 insertions(+), 1 deletions(-)
> >
> > diff --git a/hugectl.c b/hugectl.c
> > index 6742637..346d4c3 100644
> > --- a/hugectl.c
> > +++ b/hugectl.c
> > @@ -59,6 +59,7 @@ void print_usage()
> > fprintf(stderr, "options:\n");
> >
> > OPTION("--help, -h", "Prints this message");
> > + OPTION("--verbose <level>, -v", "Increases/sets tracing levels");
> >
> > OPTION("--text", "Requests remapping of the program text");
> > OPTION("--data", "Requests remapping of the program data");
> > @@ -97,6 +98,20 @@ void verbose_init(void)
> > verbose_level = VERBOSITY_MAX;
> > }
> >
> > +void verbose(char *which)
> > +{
> > + if (which)
> > + verbose_level = atoi(which);
> > + else
> > + verbose_level++;
> > +
> > + if (verbose_level < 0 || verbose_level > 99) {
> > + fprintf(stderr, "hugectl: %d: verbosity out of range 0-99\n",
> > + verbose_level);
> > + exit(EXIT_FAILURE);
> > + }
> > +}
>
> Makes sense and verbose_level should already be initialised as a
> sensible value (ERROR by the looks of things).
Yes the default for the whole library is followed automatically.
Currently errors only.
> > +
> > void setup_environment(char *var, char *val)
> > {
> > setenv(var, val, 1);
> > @@ -106,6 +121,16 @@ void setup_environment(char *var, char *val)
> > printf("%s='%s'\n", var, val);
> > }
> >
> > +void verbose_expose(void)
> > +{
> > + char level[3];
> > +
> > + if (verbose_level == 99) {
> > + setenv("HUGETLB_DEBUG", "yes", 1);
> > + }
> > + snprintf(level, sizeof(level), "%d", verbose_level);
> > + setup_environment("HUGETLB_VERBOSE", level);
> > +}
> >
> > /*
> > * getopts return values for options which are long only.
> > @@ -235,10 +260,11 @@ int main(int argc, char** argv)
> > int opt_preload = 1;
> > char *opt_library = NULL;
> >
> > - char opts[] = "+h";
> > + char opts[] = "+hv";
> > int ret = 0, index = 0;
> > struct option long_opts[] = {
> > {"help", no_argument, NULL, 'h'},
> > + {"verbose", required_argument, NULL, 'v' },
> > {"no-preload", no_argument, NULL, LONG_NO_PRELOAD},
> > {"dry-run", no_argument, NULL, LONG_DRY_RUN},
> > {"library-path",
> > @@ -272,6 +298,10 @@ int main(int argc, char** argv)
> > print_usage();
> > exit(EXIT_SUCCESS);
> >
> > + case 'v':
> > + verbose(optarg);
> > + break;
> > +
> > case LONG_NO_PRELOAD:
> > opt_preload = 0;
> > DEBUG("LD_PRELOAD disabled\n");
> > @@ -304,6 +334,8 @@ int main(int argc, char** argv)
> > exit(EXIT_FAILURE);
> > }
> >
> > + verbose_expose();
> > +
> > if (opt_library != LIBRARY_DISABLE)
> > library_path(opt_library);
> >
>
> One minor problem, we are not handling -1 being returned from getopt()
> properly. The while statment doesn't break on -1 in time and -1 looks like
> something with MAP_BASE set. With -v, I'm getting an unparsed option warning
> when testing with
>
> $ ./obj/hugectl -v -v -v -v -v printenv HUGETLB_VERBOSE HUGETLB_DEBUG
> hugectl: WARNING: unparsed option ffffffff
> hugectl: DEBUG: HUGETLB_VERBOSE='6'
> hugectl: DEBUG: LD_LIBRARY_PATH='/usr/local/lib:'
> hugectl: DEBUG: LD_PRELOAD not appropriate for this map combination
> 6
>
> Something like the following maybe?
>
> diff --git a/hugectl.c b/hugectl.c
> index 346d4c3..f749293 100644
> --- a/hugectl.c
> +++ b/hugectl.c
> @@ -283,8 +283,7 @@ int main(int argc, char** argv)
>
> verbose_init();
>
> - while (ret != -1) {
> - ret = getopt_long(argc, argv, opts, long_opts, &index);
> + while ((ret = getopt_long(argc, argv, opts, long_opts, &index)) != -1) {
> if (ret > 0 && (ret & MAP_BASE)) {
> opt_mappings |= ret;
> continue;
>
I've added a new patch to the series for this.
-apw
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Libhugetlbfs-devel mailing list
Libhugetlbfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
