Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC][PATCH] GCOV: profile by modules
To:     gengcixi@gmail.com, orsonzhai@gmail.com, zhang.lyra@gmail.com,
        gregkh@linuxfoundation.org, jslaby@suse.com,
        linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Cixi Geng <cixi.geng1@unisoc.com>
References: <20200518080703.22431-1-gengcixi@gmail.com>
From:   Peter Oberparleiter <oberpar@linux.ibm.com>
Message-ID: <017a2780-91ea-6d34-4007-959e3014eb6d@linux.ibm.com>
Date:   Mon, 18 May 2020 12:00:41 +0200
MIME-Version: 1.0
In-Reply-To: <20200518080703.22431-1-gengcixi@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 18.05.2020 10:07, gengcixi@gmail.com wrote:
> From: Cixi Geng <cixi.geng1@unisoc.com>
> 
> The CONFIG_GCOV_PROFILE_ALL will compile kernel by profiling entire
> kernel which will lead to kernel run slower.Use GCOV_PROFILE_MODULES
> to control part of the kernel modules to open gcov.
> 
> Only add SERIAL_GCOV for an example.
> 
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>

I'm not convinced that the introduction of a GCOV_PROFILE_MODULES config
symbol in this way is a good idea. The name of the symbol suggests that
this would enabled GCOV profiling for all kernel modules, which is not
the case. In the end it is only an abbreviation for adding the
dependencies required for enabling GCOV profiling for specific modules.

I'd instead suggest the introduction of an automatic symbol
GCOV_PROFILE_PREREQS (or similar) that is enabled when both GCOV_KERNEL
is set, and COMPILE_TEST is not set. There is no need to depend on
ARCH_HAS_GCOV_PROFILE_ALL. The SERIAL_GCOV symbol and other related
symbols of modules that want to introduce similar config options can
then depend on this symbol.

As a reminder, here are the involved symbols and their meaning in the
context of GCOV.

GCOV_KERNEL: You need this to get gcov data out of the kernel
!COMPILE_TESTS: You don't want to enable any GCOV profile during compile
tests due to unwanted side-effects
ARCH_HAS_GCOV_PROFILE_ALL: An architecture has ensured that enabling
full-kernel profiling does not cause any unwanted side-effects. As this
relates to kernel base code only it is not required in the context of
modules.


> ---
>  drivers/tty/serial/Kconfig  |  6 ++++++
>  drivers/tty/serial/Makefile |  1 +
>  kernel/gcov/Kconfig         | 15 +++++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index adf9e80e7dc9..0b4c070f6c74 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1566,3 +1566,9 @@ endmenu
>  
>  config SERIAL_MCTRL_GPIO
>  	tristate
> +
> +config SERIAL_GCOV
> +       bool "open gcov for serial"
> +       depends on GCOV_PROFILE_MODULES
> +       help
> +         Say Y here to enable gcov in serial.
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index d056ee6cca33..7be1202286d2 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -2,6 +2,7 @@
>  #
>  # Makefile for the kernel serial device drivers.
>  #
> +GCOV_PROFILE := $(CONFIG_SERIAL_GCOV)
>  
>  obj-$(CONFIG_SERIAL_CORE) += serial_core.o
>  
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 3941a9c48f83..7f9c1d2476c9 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -51,6 +51,21 @@ config GCOV_PROFILE_ALL
>  	larger and run slower. Also be sure to exclude files from profiling
>  	which are not linked to the kernel image to prevent linker errors.
>  
> +config GCOV_PROFILE_MODULES
> +	bool "Profile Kernel subsytem"
> +	depends on !COMPILE_TEST
> +	depends on GCOV_KERNEL
> +	depends on ARCH_HAS_GCOV_PROFILE_ALL
> +	default n
> +	help
> +	This options activates profiling for the specified kernel modules.
> +
> +	If unsure, say N.
> +
> +	When some modules need Gcov data, enable this config, then configure
> +	with gcov on the corresponding modules configs.The directories or files
> +	of these modules will be added profiling flags after kernel compile.
> +
>  choice
>  	prompt "Specify GCOV format"
>  	depends on GCOV_KERNEL
> 


-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany