[PATCH v2 0/4] gcov: more cleanup

[PATCH v2 0/4] gcov: more cleanup

[Wei Liu]

Wei Liu (4):
  xen: indicate gcov in log messages
  gcov: collect more sections to constructor list
  xen: replace TEST_COVERAGE with CONFIG_GCOV
  xen: make clear gcov support limitation in Kconfig

 xen/Kconfig.debug           | 6 ++++++
 xen/Rules.mk                | 2 +-
 xen/arch/arm/traps.c        | 5 +++--
 xen/arch/arm/xen.lds.S      | 2 ++
 xen/arch/x86/x86_64/traps.c | 5 +++--
 xen/arch/x86/xen.lds.S      | 2 ++
 xen/common/sysctl.c         | 2 +-
 xen/drivers/char/console.c  | 5 +++--
 xen/include/xen/gcov.h      | 2 +-
 xen/include/xen/lib.h       | 6 ++++++
 10 files changed, 28 insertions(+), 9 deletions(-)

-- 
2.1.4


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

[PATCH v2 1/4] xen: indicate gcov in log messages

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/arm/traps.c        | 5 +++--
 xen/arch/x86/x86_64/traps.c | 5 +++--
 xen/drivers/char/console.c  | 5 +++--
 xen/include/xen/lib.h       | 6 ++++++
 4 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 683bcb2..3bac8e8 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -141,14 +141,15 @@ 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=%c  %s ]----\n",
            xen_major_version(), xen_minor_version(), xen_extra_version(),
 #ifdef CONFIG_ARM_32
            "arm32",
 #else
            "arm64",
 #endif
-           debug_build() ? 'y' : 'n', print_tainted(taint_str));
+           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
+           print_tainted(taint_str));
 }
 
 #ifdef CONFIG_ARM_32
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 2d8ecf5..0708cc7 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -30,9 +30,10 @@ 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=%c %s ]----\n",
            xen_major_version(), xen_minor_version(), xen_extra_version(),
-           debug_build() ? 'y' : 'n', print_tainted(taint_str));
+           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
+           print_tainted(taint_str));
 }
 
 enum context { CTXT_hypervisor, CTXT_pv_guest, CTXT_hvm_guest };
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 650035d..e773076 100644
--- 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",
            xen_major_version(), xen_minor_version(), xen_extra_version(),
            xen_compile_by(), xen_compile_domain(),
-           xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
+           xen_compiler(), debug_build() ? 'y' : 'n',
+           gcov_build() ? 'y' : 'n', xen_compile_date());
     printk("Latest ChangeSet: %s\n", xen_changeset());
 
     if ( opt_sync_console )
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e518adc..464fb05 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_build() 1
+#else
+#define gcov_build() 0
+#endif
+
 #ifndef NDEBUG
 #define ASSERT(p) \
     do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)
-- 
2.1.4


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

[PATCH v2 2/4] gcov: collect more sections to constructor list

[Wei Liu]

The version of gcc (4.9.2) I use put constructors into .init_array*
section(s). Collect those sections into constructor list as well.

Modify both arm and x86 scripts to keep them in sync.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
---
v2: move .init_arry before .init_array.*

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/arm/xen.lds.S | 2 ++
 xen/arch/x86/xen.lds.S | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index b24e93b..3c5e7ba 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -166,7 +166,9 @@ SECTIONS
 
        . = ALIGN(8);
        __ctors_start = .;
+       *(.ctors)
        *(.init_array)
+       *(SORT(.init_array.*))
        __ctors_end = .;
   } :text
   __init_end_efi = .;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 67cfda1..d903c31 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -205,6 +205,8 @@ SECTIONS
        . = ALIGN(8);
        __ctors_start = .;
        *(.ctors)
+       *(.init_array)
+       *(SORT(.init_array.*))
        __ctors_end = .;
   } :text
 
-- 
2.1.4


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

[PATCH v2 3/4] xen: replace TEST_COVERAGE with CONFIG_GCOV

[Wei Liu]

The sole purpose of TEST_COVERAGE macro is to guard the availability of
gcov sysctl. Now we have a proper CONFIG_GCOV, use it.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
 xen/Rules.mk           | 2 +-
 xen/common/sysctl.c    | 2 +-
 xen/include/xen/gcov.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 696aaa8..a9fda71 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -116,7 +116,7 @@ subdir-all := $(subdir-y) $(subdir-n)
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_GCOV),y)
-$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage
 endif
 
 ifeq ($(lto),y)
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 55f2077..8aea6ef 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -396,7 +396,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     }
     break;
 
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
     case XEN_SYSCTL_coverage_op:
         ret = sysctl_coverage_op(&op->u.coverage_op);
         break;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index 27c5c37..a7d4a35 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -86,7 +86,7 @@ struct gcov_info
 /**
  * Sysctl operations for coverage
  */
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
 int sysctl_coverage_op(xen_sysctl_coverage_op_t *op);
 #endif
 
-- 
2.1.4


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

[PATCH v2 4/4] xen: make clear gcov support limitation in Kconfig

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/Kconfig.debug | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 06afd80..12a1193 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -33,6 +33,12 @@ config GCOV
        ---help---
          Enable gcov (a test coverage program in GCC) support.
 
+         Currently the data structure and hypercall interface are tied
+         to GCC 3.4 gcov format. You need to have a version of GCC
+         that is compatible with that format to make gcov work.
+
+         If unsure, say N here.
+
 config LOCK_PROFILE
 	bool "Lock Profiling"
 	---help---
-- 
2.1.4


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

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

[Andrew Cooper]

On 06/09/16 13:41, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/arm/traps.c        | 5 +++--
>  xen/arch/x86/x86_64/traps.c | 5 +++--
>  xen/drivers/char/console.c  | 5 +++--
>  xen/include/xen/lib.h       | 6 ++++++
>  4 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 683bcb2..3bac8e8 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -141,14 +141,15 @@ 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=%c  %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>  #ifdef CONFIG_ARM_32
>             "arm32",
>  #else
>             "arm64",
>  #endif
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
> +           print_tainted(taint_str));
>  }
>  
>  #ifdef CONFIG_ARM_32
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 2d8ecf5..0708cc7 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -30,9 +30,10 @@ 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=%c %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
> +           print_tainted(taint_str));
>  }
>  
>  enum context { CTXT_hypervisor, CTXT_pv_guest, CTXT_hvm_guest };
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 650035d..e773076 100644
> --- 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.)

~Andrew

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

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

[Jan Beulich]

>>> On 06.09.16 at 14:47, <andrew.cooper3@citrix.com> 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?

Jan


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

Re: [PATCH v2 2/4] gcov: collect more sections to constructor list

[Jan Beulich]

>>> On 06.09.16 at 14:41, <wei.liu2@citrix.com> wrote:
> The version of gcc (4.9.2) I use put constructors into .init_array*
> section(s). Collect those sections into constructor list as well.
> 
> Modify both arm and x86 scripts to keep them in sync.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Julien Grall <julien.grall@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

Re: [PATCH v2 4/4] xen: make clear gcov support limitation in Kconfig

[Jan Beulich]

>>> On 06.09.16 at 14:41, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

with one possible further request:

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -33,6 +33,12 @@ config GCOV
>         ---help---
>           Enable gcov (a test coverage program in GCC) support.
>  
> +         Currently the data structure and hypercall interface are tied
> +         to GCC 3.4 gcov format. You need to have a version of GCC
> +         that is compatible with that format to make gcov work.

Is it known up to which gcc version that format did get used? I ask
because mentioning 3.4 when our minimum requirement is 4.1 now
is a little odd. If the format had changed by 4.1, we may want to
consider to make GCOV depend on BROKEN.

Jan


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

Re: [PATCH v2 4/4] xen: make clear gcov support limitation in Kconfig

[Andrew Cooper]

On 06/09/16 14:37, Jan Beulich wrote:
>>>> On 06.09.16 at 14:41, <wei.liu2@citrix.com> wrote:
>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
>
> with one possible further request:
>
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -33,6 +33,12 @@ config GCOV
>>         ---help---
>>           Enable gcov (a test coverage program in GCC) support.
>>  
>> +         Currently the data structure and hypercall interface are tied
>> +         to GCC 3.4 gcov format. You need to have a version of GCC
>> +         that is compatible with that format to make gcov work.
> Is it known up to which gcc version that format did get used? I ask
> because mentioning 3.4 when our minimum requirement is 4.1 now
> is a little odd. If the format had changed by 4.1, we may want to
> consider to make GCOV depend on BROKEN.

The GCC GCOV version is not the GCC Version.

GCC GCOV version 3.4 is used up until GCC 4.3 or 4.4 (can't remember
which).  The previous GCOV support in Xen definitely did function
properly with GCC 4.1.

~Andrew

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

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

[Wei Liu]

On Tue, Sep 06, 2016 at 07:34:26AM -0600, Jan Beulich wrote:
> >>> On 06.09.16 at 14:47, <andrew.cooper3@citrix.com> 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@citrix.com>
Date: Fri, 2 Sep 2016 14:43:25 +0100
Subject: [PATCH] xen: indicate gcov in log messages

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v3: simplify code

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 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)
-- 
2.1.4


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

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

[Jan Beulich]

>>> On 06.09.16 at 16:06, <wei.liu2@citrix.com> wrote:
> On Tue, Sep 06, 2016 at 07:34:26AM -0600, Jan Beulich wrote:
>> >>> On 06.09.16 at 14:47, <andrew.cooper3@citrix.com> 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

And this way it can have my ack.

Jan


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

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

[Andrew Cooper]

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@citrix.com> 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@citrix.com>
> Date: Fri, 2 Sep 2016 14:43:25 +0100
> Subject: [PATCH] xen: indicate gcov in log messages
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

> ---
> v3: simplify code
>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Stefano Stabellini]

On Tue, 6 Sep 2016, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/arm/traps.c        | 5 +++--
>  xen/arch/x86/x86_64/traps.c | 5 +++--
>  xen/drivers/char/console.c  | 5 +++--
>  xen/include/xen/lib.h       | 6 ++++++
>  4 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 683bcb2..3bac8e8 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -141,14 +141,15 @@ 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=%c  %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>  #ifdef CONFIG_ARM_32
>             "arm32",
>  #else
>             "arm64",
>  #endif
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
> +           print_tainted(taint_str));
>  }
>  
>  #ifdef CONFIG_ARM_32
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 2d8ecf5..0708cc7 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -30,9 +30,10 @@ 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=%c %s ]----\n",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
> -           debug_build() ? 'y' : 'n', print_tainted(taint_str));
> +           debug_build() ? 'y' : 'n', gcov_build() ? 'y' : 'n',
> +           print_tainted(taint_str));
>  }
>  
>  enum context { CTXT_hypervisor, CTXT_pv_guest, CTXT_hvm_guest };
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 650035d..e773076 100644
> --- 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",
>             xen_major_version(), xen_minor_version(), xen_extra_version(),
>             xen_compile_by(), xen_compile_domain(),
> -           xen_compiler(), debug_build() ? 'y' : 'n', xen_compile_date());
> +           xen_compiler(), debug_build() ? 'y' : 'n',
> +           gcov_build() ? 'y' : 'n', xen_compile_date());
>      printk("Latest ChangeSet: %s\n", xen_changeset());
>  
>      if ( opt_sync_console )
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index e518adc..464fb05 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_build() 1
> +#else
> +#define gcov_build() 0
> +#endif
> +
>  #ifndef NDEBUG
>  #define ASSERT(p) \
>      do { if ( unlikely(!(p)) ) assert_failed(#p); } while (0)
> -- 
> 2.1.4
> 

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

Re: [PATCH v2 0/4] gcov: more cleanup

[Wei Liu]

On Tue, Sep 06, 2016 at 01:41:03PM +0100, Wei Liu wrote:
> Wei Liu (4):
>   xen: indicate gcov in log messages
>   gcov: collect more sections to constructor list
>   xen: replace TEST_COVERAGE with CONFIG_GCOV
>   xen: make clear gcov support limitation in Kconfig
> 

I believe this series has got sufficient reviews / acks and all
questions answered (with help from Andrew), so I pushed it just now.

Wei.

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