On Fri, Nov 14, 2008 at 09:00:32PM +0000, Adam Litke wrote:
> The default verbosity level in libhugetlbfs is 1, meaning that only errors are
> printed. Many of the new library features can have outcomes that are very
> meaningful warnings which should be displayed by default. For example, if a
> user specifies an incorrect page size in HUGETLB_ELFMAP, a warning is issued
> and segment remapping is disabled. Under the current verbosity default, this
> important message is suppressed. We should switch the default verbosity to 2
> so that such warnings are displayed.
>
> Simply increasing the default verbosity is not a complete solution. Some
> messages that have been classified as warnings are more informational in
> nature
> and should not always be printed. Add a new INFO verbosity level for detailed
> messages about normal library operation.
>
> The library currently makes a strange differentiation between verbosity levels
> and whether debugging mode is enabled. Verbosity is controlled by one
> enviroment variable and debugging by another. Thus, a user could set
> verbosity
> to 0 (no messages) and enable debugging (resulting in output with out of place
> debugging info and non-obvious slow downs.
>
> One can argue that debugging mode is equivalent to the highest verbosity
> level.
> Making the switch simplifies the library and interface.
>
Did we not separate out debug in case debug enabling took additional
debug-only steps? i.e. debug verbosity may mean printing out of more
messages like trace messages but debug-enabled made additional checks?
> So after this patch is applied, we have the following four verbosity levels
> with their intended uses as defined herein.
>
> ERROR - Severe, unrecoverable errors
> WARNING - Recoverable condition, but may result in altered semantics
> INFO - Detailed information about normal library operations
> DEBUG - Diagnostic information for debugging, potential runtime overhead
>
> Signed-off-by: Adam Litke <[EMAIL PROTECTED]>
> ---
>
> debug.c | 3 +--
> libhugetlbfs_debug.h | 17 +++++++++++++----
> libhugetlbfs_internal.h | 21 +++++++++++++++------
> 3 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/debug.c b/debug.c
> index e854dff..7842115 100644
> --- a/debug.c
> +++ b/debug.c
> @@ -28,7 +28,6 @@
> #include "libhugetlbfs_internal.h"
>
> int __hugetlbfs_verbose = VERBOSITY_DEFAULT;
> -int __hugetlbfs_debug = 0;
> int __hugetlbfs_prefault = 1;
> char __hugetlbfs_hostname[64];
>
> @@ -47,7 +46,7 @@ static void __hugetlbfs_init_debug(void)
>
> env = getenv("HUGETLB_DEBUG");
> if (env)
> - __hugetlbfs_debug = 1;
> + __hugetlbfs_verbose = VERBOSE_DEBUG;
>
> env = getenv("HUGETLB_NO_PREFAULT");
> if (env)
> diff --git a/libhugetlbfs_debug.h b/libhugetlbfs_debug.h
> index 7eb8f4e..cd490ad 100644
> --- a/libhugetlbfs_debug.h
> +++ b/libhugetlbfs_debug.h
> @@ -20,14 +20,23 @@
> #ifndef _LIBHUGETLBFS_DEBUG_H
> #define _LIBHUGETLBFS_DEBUG_H
>
> +/* Severe, unrecoverable errors */
> #define ERROR(...) REPORT(1, "ERROR", ##__VA_ARGS__)
> #define ERROR_CONT(...) REPORT_CONT(1, "ERROR", ##__VA_ARGS__)
> +
> +/* A condition that is recoverable, but may result in altered semantics */
> #define WARNING(...) REPORT(2, "WARNING", ##__VA_ARGS__)
> #define WARNING_CONT(...) REPORT_CONT(2, "WARNING", ##__VA_ARGS__)
> -#define DEBUG(...) REPORT(3, "DEBUG", ##__VA_ARGS__)
> -#define DEBUG_CONT(...) REPORT_CONT(3, "DEBUG", ##__VA_ARGS__)
>
> -#define VERBOSITY_MAX 3
> -#define VERBOSITY_DEFAULT 1
> +/* Detailed information about normal library operations */
> +#define INFO(...) REPORT(3, "INFO", ##__VA_ARGS__)
> +#define INFO_CONT(...) REPORT_CONT(3, "INFO", ##__VA_ARGS__)
> +
> +/* Diagnostic information used for debugging problems */
> +#define DEBUG(...) REPORT(4, "DEBUG", ##__VA_ARGS__)
> +#define DEBUG_CONT(...) REPORT_CONT(4, "DEBUG", ##__VA_ARGS__)
> +
> +#define VERBOSITY_MAX 4
> +#define VERBOSITY_DEFAULT 2
>
> #endif
> diff --git a/libhugetlbfs_internal.h b/libhugetlbfs_internal.h
> index 6ebe9c0..8bd979b 100644
> --- a/libhugetlbfs_internal.h
> +++ b/libhugetlbfs_internal.h
> @@ -66,8 +66,6 @@
> */
> #define __hugetlbfs_verbose __lh___hugetlbfs_verbose
> extern int __hugetlbfs_verbose;
> -#define __hugetlbfs_debug __lh___hugetlbfs_debug
> -extern int __hugetlbfs_debug;
> #define __hugetlbfs_prefault __lh___hugetlbfs_prefault
> extern int __hugetlbfs_prefault;
> #define hugetlbfs_setup_elflink __lh_hugetlbfs_setup_elflink
> @@ -90,12 +88,23 @@ extern long parse_page_size(const char *str);
> #ifndef REPORT_UTIL
> #define REPORT_UTIL "libhugetlbfs"
> #endif
> +
> +#define VERBOSE_ERROR 1
> +#define VERBOSE_WARNING 2
> +#define VERBOSE_INFO 3
> +#define VERBOSE_DEBUG 4
> +
> +#define __hugetlbfs_debug (__hugetlbfs_verbose >= VERBOSE_DEBUG)
> +
> #ifndef REPORT
> #define REPORT(level, prefix, format, ...) \
> do { \
> - if (__hugetlbfs_debug || __hugetlbfs_verbose >= level) { \
> - fprintf(stderr, REPORT_UTIL " [%s:%d]: " prefix ": " \
> - format, __hugetlbfs_hostname, getpid(), \
> + if (__hugetlbfs_verbose >= level) { \
> + fprintf(stderr, REPORT_UTIL); \
> + if (__hugetlbfs_verbose >= VERBOSE_DEBUG) \
> + fprintf(stderr, " [%s:%d]", \
> + __hugetlbfs_hostname, getpid()); \
> + fprintf(stderr, ": " prefix ": " format, \
hmm, this is actually a semantic change. Previously, we always printed
the hostname and pid. Now, we only do it when verbosity is set to debug.
That might be confusing for parsers of logs for example and I'm failing
to see the benefit of the change. In addition, one fprintf is replaced
by three plus a branch. What is the thinking behind this change?
> ##__VA_ARGS__); \
> fflush(stderr); \
> } \
> @@ -103,7 +112,7 @@ extern long parse_page_size(const char *str);
>
> #define REPORT_CONT(level, prefix, ...) \
> do { \
> - if (__hugetlbfs_debug || __hugetlbfs_verbose >= level) { \
> + if (__hugetlbfs_verbose >= level) { \
> fprintf(stderr, ##__VA_ARGS__); \
> fflush(stderr); \
> } \
>
>
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
-------------------------------------------------------------------------
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
