Re: [Xen-devel] [PATCH v2 1/4] xen: indicate gcov in log messages

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/09/16 15:06, Wei Liu wrote:
> On Tue, Sep 06, 2016 at 07:34:26AM -0600, Jan Beulich wrote:
>>>>> On 06.09.16 at 14:47, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 06/09/16 13:41, Wei Liu wrote:
>>>> --- a/xen/drivers/char/console.c
>>>> +++ b/xen/drivers/char/console.c
>>>> @@ -735,10 +735,11 @@ void __init console_init_preirq(void)
>>>>      spin_lock(&console_lock);
>>>>      __putstr(xen_banner());
>>>>      spin_unlock(&console_lock);
>>>> -    printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c %s\n",
>>>> +    printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c gcov=%c %s\n",
>>> Please instead use
>>>
>>> printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c"
>>> #ifdef CONFIG_GCOV
>>> " gcov=y"
>>> #endif
>>> " %s\n",
>>>
>>> Enabling gcov will be extremely rare in the grand scheme of things, and
>>> this method causes zero overhead for the case where gcov is compiled
>>> out.  (Similarly elsewhere.)
>> How about instead of the kind of ugly #ifdef-ery above, instead of
>>
>> +#ifdef CONFIG_GCOV
>> +#define gcov_build() 1
>> +#else
>> +#define gcov_build() 0
>> +#endif
>>
>> we'd have
>>
>> +#ifdef CONFIG_GCOV
>> +#define gcov_string "gcov=y"
>> +#else
>> +#define gcov_string ""
>> +#endif
>>
>> (with the identifier name open for improvement), which can then be
>> embedded easily into any string literal?
>>
> Now this patch looks like
>
> ---8<---
> From 56718288f632d3a25bd3fe89777df99008a13d6e Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@xxxxxxxxxx>
> Date: Fri, 2 Sep 2016 14:43:25 +0100
> Subject: [PATCH] xen: indicate gcov in log messages
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> ---
> v3: simplify code
>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  xen/arch/arm/traps.c        | 2 +-
>  xen/arch/x86/x86_64/traps.c | 2 +-
>  xen/drivers/char/console.c  | 2 +-
>  xen/include/xen/lib.h       | 6 ++++++
>  4 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 9353ee7..39a05fd 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -141,7 +141,7 @@ static void print_xen_info(void)
>  {
>      char taint_str[TAINT_STRING_MAX_LEN];
>  
> -    printk("----[ Xen-%d.%d%s  %s  debug=%c  %s ]----\n",
> +    printk("----[ Xen-%d.%d%s  %s  debug=%c " gcov_string "  %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>  #ifdef CONFIG_ARM_32
>             "arm32",
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 16de0be..fc8cde6 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -31,7 +31,7 @@ static void print_xen_info(void)
>  {
>      char taint_str[TAINT_STRING_MAX_LEN];
>  
> -    printk("----[ Xen-%d.%d%s  x86_64  debug=%c  %s ]----\n",
> +    printk("----[ Xen-%d.%d%s  x86_64  debug=%c " gcov_string "  %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>             debug_build() ? 'y' : 'n', print_tainted(taint_str));
>  }
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 650035d..55ae31a 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -735,7 +735,7 @@ void __init console_init_preirq(void)
>      spin_lock(&console_lock);
>      __putstr(xen_banner());
>      spin_unlock(&console_lock);
> -    printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c %s\n",
> +    printk("Xen version %d.%d%s (%s@%s) (%s) debug=%c " gcov_string " %s\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>             xen_compile_by(), xen_compile_domain(),
>             xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index e518adc..d1171b7 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -26,6 +26,12 @@
>  #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>  #endif
>  
> +#ifdef CONFIG_GCOV
> +#define gcov_string "gcov=y"
> +#else
> +#define gcov_string ""
> +#endif
> +
>  #ifndef NDEBUG
>  #define ASSERT(p) \
>      do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel