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 v4] GCOV: profile by modules
To:     gengcixi@gmail.com, gregkh@linuxfoundation.org, jslaby@suse.com,
        linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     orsonzhai@gmail.com, zhang.lyra@gmail.com,
        Cixi Geng <cixi.geng1@unisoc.com>
References: <20200525100054.17700-1-gengcixi@gmail.com>
From:   Peter Oberparleiter <oberpar@linux.ibm.com>
Message-ID: <917da283-5f1e-3fdb-13bb-4251312dc77a@linux.ibm.com>
Date:   Thu, 28 May 2020 16:47:38 +0200
MIME-Version: 1.0
In-Reply-To: <20200525100054.17700-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 25.05.2020 12:00, 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_PREREQS
> to control part of the kernel modules to open gcov.

Maybe rephrase the commit message to make the intention more obvious:

Introduce new configuration option GCOV_PROFILE_PREREQS that can be used
to check whether the prerequisites for enabling gcov profiling for
specific files and directories are met.

> 
> Only add SERIAL_GCOV for an example.
> 
> Signed-off-by: Cixi Geng <cixi.geng1@unisoc.com>
> ---
>  drivers/tty/serial/Kconfig  |  7 +++++++
>  drivers/tty/serial/Makefile |  1 +
>  kernel/gcov/Kconfig         | 14 ++++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index adf9e80e7dc9..6df002370f18 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1566,3 +1566,10 @@ endmenu
>  
>  config SERIAL_MCTRL_GPIO
>  	tristate
> +
> +config SERIAL_GCOV
> +	bool "Enable profile gcov for serial directory"
> +	depends on GCOV_PROFILE_PREREQS
> +	help
> +	  The SERIAL_GCOV will add Gcov profiling flags when kernel compiles.
> +	  Say 'Y' here if you want the gcov data for the serial directory,
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index d056ee6cca33..17272733db95 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for the kernel serial device drivers.
>  #
>  
> +GCOV_PROFILE := $(CONFIG_SERIAL_GCOV)
>  obj-$(CONFIG_SERIAL_CORE) += serial_core.o
>  
>  obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 3941a9c48f83..ea8b514f5676 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -51,6 +51,20 @@ 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_PREREQS
> +	bool "Profile Kernel subsytem"
> +	depends on !COMPILE_TEST
> +	depends on GCOV_KERNEL
> +	depends on !GCOV_PROFILE_ALL
> +	help
> +	  This options activates profiling for the specified kernel modules.
> +
> +	  When some modules need Gcov data, enable this config, then configure
> +	  with gcov on the corresponding modules,The directories or files of
> +	  these modules will be added profiling flags after kernel compile.
> +
> +	  If unsure, say N.

This is not quite what I had in mind. A user still needs to manually
select GCOV_PROFILE, then GCOV_PROFILE_PREREQS, then SERIAL_GCOV.

My though was that you could provide an automatic config option that is
invisible in the configuration dialog, and that is automatically
selected when all prereqs for enabling GCOV in a module directory are met.

Something like the following:

config GCOV_PROFILE_PREREQS
        bool
        default y if GCOV_KERNEL && !COMPILE_TEST
        default n

There's no need to add GCOV_KERNEL_ALL here since that is about a user
choice ("I want all code compiled with profiling") vs. this new symbol
which is about an ability ("all prereqs for enabling profiling in
specific directories are met).


> +
>  choice
>  	prompt "Specify GCOV format"
>  	depends on GCOV_KERNEL
> 


-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany