On 16.09.2020 14:55, Cixi Geng wrote: > Hi ALL： > Not recieve more advise for a long time , > Can this submission be merged recently? First off, sorry for not replying earlier. I tried out your latest version of this patch and I don't see that my previous comments have been addressed. To re-iterate my point: I only see value in introducing a new GCOV-related config symbol that is automatically selected, depending on whether (as the name implies) all prerequisites for enabling GCOV-based kernel profiling have been met. Such a symbol can take away the burden of duplicating the prerequisite check as has been implemented for GCOV_PROFILE_ALL. I see no value in introducing a new config symbol that prompts the user for a choice. As it is, your patch introduces a new config symbol that prompts the user for a choice: $ make oldconfig scripts/kconfig/conf --oldconfig Kconfig * * Restart config... * * * GCOV-based kernel profiling * Enable gcov-based kernel profiling (GCOV_KERNEL) [Y/n/?] y Profile entire Kernel (GCOV_PROFILE_ALL) [N/y/?] n Profile Kernel for prereqs (GCOV_PROFILE_PREREQS) [Y/n/?] (NEW) We do not need this prompt. Users specify that they want GCOV-profiling by selecting GCOV_KERNEL. They specify that they want area-specific profiling in symbols like your proposed SERIAL_GCOV. There is no need for a user to manually confirm that the prerequisites for enabling are-specific profiling are met. I have detailed the required changes that would remove the prompt in my previous reply. I'll add it here again for your convenience: >>> +++ b/kernel/gcov/Kconfig >>> @@ -51,6 +51,16 @@ 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 for prereqs" >>> + default y if GCOV_KERNEL && !COMPILE_TEST >>> + 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. >>> + > > Replace the portion above with these lines: > > config GCOV_PROFILE_PREREQS > def_bool y if GCOV_KERNEL && !COMPILE_TEST And to clarify: by "the portion above" I was referring to all quoted lines prefixed with a '+' sign. > > Cixi Geng <gengcixi@xxxxxxxxx> 于2020年8月20日周四 下午8:40写道： >> >> Hi All: >> >> Does this patch need more modification? >> >> <gengcixi@xxxxxxxxx> 于2020年7月27日周一 下午4:51写道： >>> >>> From: Cixi Geng <cixi.geng1@xxxxxxxxxx> >>> >>> 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@xxxxxxxxxx> >>> --- >>> drivers/tty/serial/Kconfig | 7 +++++++ >>> drivers/tty/serial/Makefile | 1 + >>> kernel/gcov/Kconfig | 12 ++++++++++++ >>> 3 files changed, 20 insertions(+) >>> >>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig >>> index 780908d43557..55b128b6b31d 100644 >>> --- a/drivers/tty/serial/Kconfig >>> +++ b/drivers/tty/serial/Kconfig >>> @@ -1576,3 +1576,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 3110c77230c7..bb2e1fb85743 100644 >>> --- a/kernel/gcov/Kconfig >>> +++ b/kernel/gcov/Kconfig >>> @@ -51,4 +51,16 @@ 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 for prereqs" >>> + depends on GCOV_KERNEL >>> + depends on !COMPILE_TEST >>> + def_bool y if GCOV_KERNEL && !COMPILE_TEST >>> + 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. >>> + >>> endmenu >>> -- >>> 2.17.1 >>> -- Peter Oberparleiter Linux on Z Development - IBM Germany
