diff mbox series

[4/6] um: split up CONFIG_GCOV

Message ID 20210312104627.927fb4c7d36f.Idb980393c41c2129ee592de4ed71e7a5518212f9@changeid
State Rejected
Headers show
Series um: fix up CONFIG_GCOV support | expand

Commit Message

Johannes Berg March 12, 2021, 9:55 a.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

It's not always desirable to collect coverage data for the
entire kernel, so split off CONFIG_GCOV_BASE. This option
only enables linking with coverage options, and compiles a
single file (reboot.c) with them as well to force gcov to
be linked into the kernel binary. That way, modules also
work.

To use this new option properly, one needs to manually add
'-fprofile-arcs -ftest-coverage' to the compiler options
of some object(s) or subdir(s) to collect coverage data at
the desired places.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/Kconfig.debug   | 38 ++++++++++++++++++++++++++++++--------
 arch/um/Makefile-skas   |  2 +-
 arch/um/kernel/Makefile | 11 ++++++++++-
 3 files changed, 41 insertions(+), 10 deletions(-)

Comments

Brendan Higgins March 18, 2021, 9:27 p.m. UTC | #1 
On Fri, Mar 12, 2021 at 1:56 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> From: Johannes Berg <johannes.berg@intel.com>
>
> It's not always desirable to collect coverage data for the
> entire kernel, so split off CONFIG_GCOV_BASE. This option
> only enables linking with coverage options, and compiles a
> single file (reboot.c) with them as well to force gcov to
> be linked into the kernel binary. That way, modules also
> work.
>
> To use this new option properly, one needs to manually add
> '-fprofile-arcs -ftest-coverage' to the compiler options
> of some object(s) or subdir(s) to collect coverage data at
> the desired places.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Hey, thanks for doing this! I was looking into this a few weeks ago
and root caused part of the issue in GCC and in the kernel, but I did
not have a fix put together.

Anyway, most of the patches make sense to me, but I am not able to
apply this patch on torvalds/master. Do you mind sending a rebase so I
can test it?

Thanks!
Johannes Berg March 18, 2021, 9:30 p.m. UTC | #2 
Hi Brendan,

> Hey, thanks for doing this! I was looking into this a few weeks ago
> and root caused part of the issue in GCC and in the kernel, but I did
> not have a fix put together.
> 
> Anyway, most of the patches make sense to me, but I am not able to
> apply this patch on torvalds/master. Do you mind sending a rebase so I
> can test it?

Well, if you see my other replies in the thread, I gave up for various
reasons, see

https://lore.kernel.org/r/d36ea54d8c0a8dd706826ba844a6f27691f45d55.camel@sipsolutions.net

Personally, I ended up switching to CONFIG_GCOV_KERNEL instead because
it actually works for modules, but then it was _really_ slow (think 30s
to copy data for a few modules), but I root-caused this and ultimately
sent these patches instead:

https://patchwork.ozlabs.org/project/linux-um/patch/20210315233804.d3e52f6a3422.I9672eef7dfa7ce6c3de1ccf7ab8d9aad1fa7f3a6@changeid/
https://patchwork.ozlabs.org/project/linux-um/patch/20210315234731.2e03184a344b.I04f1816296f04c5aa7d7d88b33bd4a14dd458da8@changeid/


johannes
diff mbox series

Patch

diff --git a/arch/um/Kconfig.debug b/arch/um/Kconfig.debug
index 315d368e63ad..ca040b4e86e5 100644
--- a/arch/um/Kconfig.debug
+++ b/arch/um/Kconfig.debug
@@ -13,19 +13,41 @@  config GPROF
 	  If you're involved in UML kernel development and want to use gprof,
 	  say Y.  If you're unsure, say N.
 
-config GCOV
-	bool "Enable gcov support"
+config GCOV_BASE
+	bool "Enable gcov support (selectively)"
 	depends on DEBUG_INFO
-	depends on !KCOV
+	depends on !KCOV && !GCOV_KERNEL
 	help
 	  This option allows developers to retrieve coverage data from a UML
-	  session.
+	  session, stored to disk just like with a regular userspace binary,
+	  use the same tools (gcov, lcov, ...) to collect and process the
+	  data.
 
-	  See <http://user-mode-linux.sourceforge.net/old/gprof.html> for more
-	  details.
+	  See also KCOV and GCOV_KERNEL as alternatives.
 
-	  If you're involved in UML kernel development and want to use gcov,
-	  say Y.  If you're unsure, say N.
+	  This option (almost) only links with the needed support code, but
+	  doesn't enable coverage data collection for any code (other than a
+	  dummy file to get everything linked properly). See also the GCOV
+	  option which enables coverage collection for the entire kernel and
+	  all modules.
+
+	  If you're using UML to test something and want to manually instruct
+	  the compiler to instrument only parts of the code by adding the
+	  relevant options for the objects you care about, say Y and do that
+	  to get coverage collection only for the parts you need.
+
+	  If you're unsure, say N.
+
+config GCOV
+	bool "Enable gcov support (whole kernel)"
+	depends on DEBUG_INFO
+	depends on !KCOV && !GCOV_KERNEL
+	select GCOV_BASE
+	help
+	  This enables coverage data collection for the entire kernel and
+	  all modules, see the GCOV_BASE option for more information.
+
+	  If you're unsure, say N.
 
 config EARLY_PRINTK
 	bool "Early printk"
diff --git a/arch/um/Makefile-skas b/arch/um/Makefile-skas
index ac35de5316a6..b5be5f55ac11 100644
--- a/arch/um/Makefile-skas
+++ b/arch/um/Makefile-skas
@@ -8,5 +8,5 @@  GCOV_OPT += -fprofile-arcs -ftest-coverage
 
 CFLAGS-$(CONFIG_GCOV) += $(GCOV_OPT)
 CFLAGS-$(CONFIG_GPROF) += $(GPROF_OPT)
-LINK-$(CONFIG_GCOV) += $(GCOV_OPT)
+LINK-$(CONFIG_GCOV_BASE) += $(GCOV_OPT)
 LINK-$(CONFIG_GPROF) += $(GPROF_OPT)
diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile
index c1205f9ec17e..0403e329f931 100644
--- a/arch/um/kernel/Makefile
+++ b/arch/um/kernel/Makefile
@@ -21,7 +21,7 @@  obj-y = config.o exec.o exitcode.o irq.o ksyms.o mem.o \
 
 obj-$(CONFIG_BLK_DEV_INITRD) += initrd.o
 obj-$(CONFIG_GPROF)	+= gprof_syms.o
-obj-$(CONFIG_GCOV)	+= gmon_syms.o
+obj-$(CONFIG_GCOV_BASE)	+= gmon_syms.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
 obj-$(CONFIG_STACKTRACE) += stacktrace.o
 obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o
@@ -32,6 +32,15 @@  include arch/um/scripts/Makefile.rules
 
 targets := config.c config.tmp
 
+# This just causes _something_ to be compiled with coverage
+# collection so that gcov is linked into the binary, in case
+# the only thing that has it enabled is a module, when only
+# CONFIG_GCOV_BASE is selected. Yes, we thus always get some
+# coverage data for this file, but it's not hit often ...
+ifeq ($(CONFIG_GCOV_BASE),y)
+CFLAGS_reboot.o += -fprofile-arcs -ftest-coverage
+endif
+
 # Be careful with the below Sed code - sed is pitfall-rich!
 # We use sed to lower build requirements, for "embedded" builders for instance.