On Fri, Nov 24, 2023 at 11:02:00AM +0900, Masahiro Yamada wrote: > On Thu, Nov 23, 2023 at 8:55 AM Kent Overstreet > <kent.overstreet@xxxxxxxxx> wrote: > > > > This allows gcov to be enabled for a particular kernel source > > subdirectory on the command line, without editing makefiles, like so: > > > > make GCOV_PROFILE_fs_bcachefs=y > > > > Cc: Masahiro Yamada <masahiroy@xxxxxxxxxx> > > Cc: Nathan Chancellor <nathan@xxxxxxxxxx> > > Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx> > > Cc: Nicolas Schier <nicolas@xxxxxxxxx> > > Cc: linux-kbuild@xxxxxxxxxxxxxxx > > Signed-off-by: Kent Overstreet <kent.overstreet@xxxxxxxxx> > > --- > > scripts/Kbuild.include | 10 ++++++++++ > > scripts/Makefile.lib | 2 +- > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > > index 7778cc97a4e0..5341736f2e30 100644 > > --- a/scripts/Kbuild.include > > +++ b/scripts/Kbuild.include > > @@ -277,3 +277,13 @@ ifneq ($(and $(filter notintermediate, $(.FEATURES)),$(filter-out 4.4,$(MAKE_VER > > else > > .SECONDARY: > > endif > > + > > + # expand_parents(a/b/c) = a/b/c a/b a > > +expand_parents2 = $(if $(subst .,,$(1)),$(call expand_parents,$(1)),) > > +expand_parents = $(1) $(call expand_parents2,$(patsubst %/,%,$(dir $(1)))) > > + > > +# flatten_dirs(a/b/c) = a_b_c a_b a > > +flatten_dirs = $(subst /,_,$(call expand_parents,$(1))) > > + > > +# eval_vars(X_,a/b/c) = $(X_a_b_c) $(X_a_b) $(X_a) > > +eval_vars = $(foreach var,$(call flatten_dirs,$(2)),$($(1)$(var))) > > > > I do not like tricky code like this. > > Also, with "fs_bcachefs", it is unclear which directory > is enabled. It's consistent with how we can specify options in makefiles for a particular file. I suppose CONFIG_GCOV_PROFILE_DIRS would be fine, but your patch isn't complete so I can't test it. > > > > > How about this? > > > > [1] Specify the list of directories by GCOV_PROFILE_DIRS > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1a965fe68e01..286a569556b3 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -147,8 +147,12 @@ _cpp_flags = $(KBUILD_CPPFLAGS) $(cppflags-y) > $(CPPFLAGS_$(target-stem).lds) > # (in this order) > # > ifeq ($(CONFIG_GCOV_KERNEL),y) > +ifneq ($(filter $(obj),$(GCOV_PROFILE_DIRS)),) > +export GCOV_PROFILE_SUBDIR := y > +endif > + > _c_flags += $(if $(patsubst n%,, \ > - > $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), > \ > + > $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(GCOV_PROFILE_SUBDIR)$(CONFIG_GCOV_PROFILE_ALL)), > \ > $(CFLAGS_GCOV)) > endif > > > > Usage: > > $ make GCOV_PROFILE_DIRS=fs/bcachefs > > -> enable GCOV in fs/bachefs and its subdirectories. > > or > > $ make GCOV_PROFILE_DIRS="drivers/gpio drivers/pinctrl" > > -> enable GCOV in drivers/gpio, drivers/pinctrl, and their subdirectories. > > > > > [2] Do equivalent, but from a CONFIG option > > > config GCOV_PROFILE_DIRS > string "Directories to enable GCOV" > > > Then, you can set CONFIG_GCOV_PROFILE_DIRS="fs/bcachefs" > > > This might be a more natural approach because we already have > CONFIG_GCOV_PROFILE_ALL, although it might eventually go away > because CONFIG_GCOV_PROFILE_ALL=y is almost equivalent to > CONFIG_GCOV_PROFILE_DIRS="." > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1a965fe68e01..286a569556b3 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -147,8 +147,12 @@ _cpp_flags = $(KBUILD_CPPFLAGS) $(cppflags-y) > $(CPPFLAGS_$(target-stem).lds) > # (in this order) > # > ifeq ($(CONFIG_GCOV_KERNEL),y) > +ifneq ($(filter $(obj),$(CONFIG_GCOV_PROFILE_DIRS)),) > +export GCOV_PROFILE_SUBDIR := y > +endif > + > _c_flags += $(if $(patsubst n%,, \ > - > $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), > \ > + > $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(GCOV_PROFILE_SUBDIR)$(CONFIG_GCOV_PROFILE_ALL)), > \ > $(CFLAGS_GCOV)) > endif > > > > > > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index 1a965fe68e01..0b4581a8bc33 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -148,7 +148,7 @@ _cpp_flags = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(target-stem).lds) > > # > > ifeq ($(CONFIG_GCOV_KERNEL),y) > > _c_flags += $(if $(patsubst n%,, \ > > - $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), \ > > + $(GCOV_PROFILE_$(basetarget).o)$(call eval_vars,GCOV_PROFILE_,$(src))$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)), \ > > $(CFLAGS_GCOV)) > > endif > > > > -- > > 2.42.0 > > > > > -- > Best Regards > Masahiro Yamada
