From: Kent Overstreet <kent.overstreet@linux.dev>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>,
Nick Desaulniers <ndesaulniers@google.com>,
Nathan Chancellor <nathan@kernel.org>,
linux-kbuild@vger.kernel.org
Subject: Re: Specifying CFLAGS for a directory on the command line
Date: Thu, 15 Jun 2023 10:22:11 -0400 [thread overview]
Message-ID: <ZIsek6ChkwkTHf2+@moria.home.lan> (raw)
In-Reply-To: <CAK7LNASjy5Gb31rNx4aqLzqmR01b8YYkOFDwD8L93uYmQnzrKw@mail.gmail.com>
On Thu, Jun 15, 2023 at 07:39:08PM +0900, Masahiro Yamada wrote:
> On Thu, Jun 15, 2023 at 6:54 PM Peter Oberparleiter
> <oberpar@linux.ibm.com> wrote:
> >
> > On 13.06.2023 20:22, Kent Overstreet wrote:
> > > On Mon, Jun 12, 2023 at 06:18:35PM +0200, Peter Oberparleiter wrote:
> > >> I'm unaware of any kbuild support for setting GCOV_PROFILE for a
> > >> specific sub-directory from the command line, only from within the
> > >> associated Makefile. I'm not sure how this could have worked in the past
> > >> with the provide sample command line.
> > >>
> > >> Here's how GCOV_PROFILE evaluation works (from scripts/Makefile.lib):
> > >>
> > >> ifeq ($(CONFIG_GCOV_KERNEL),y)
> > >> _c_flags += $(if $(patsubst n%,, \
> > >> $(GCOV_PROFILE_$(basetarget).o)$(GCOV_PROFILE)$(CONFIG_GCOV_PROFILE_ALL)),\
> > >> $(CFLAGS_GCOV))
> > >> endif
> > >>
> > >> This bit of Makefile code determines whether to add the flags needed to
> > >> enabled gcov profiling (CFLAGS_GCOV) to the compiler flags for the
> > >> current compilation unit (_c_flags) by looking at the concatenation of
> > >> the following variables:
> > >>
> > >> - GCOV_PROFILE_<target base name>.o
> > >> - GCOV_PROFILE
> > >> - CONFIG_GCOV_PROFILE_ALL
> > >>
> > >> gcov flags are only added if this concatenation does not start with an
> > >> "n", and at least one of these variables is set to a non-empty value
> > >> other than "n" ("y" typically). The "starts with" part is required to
> > >> enable precedence for the more specific variable, e.g. an "n" in
> > >> GCOV_PROFILE_filename.o overwrites a "y" in GCOV_PROFILE.
> > >>
> > >> As you can see, there is no reference to a GCOV_PROFILE variable that is
> > >> named after the sub-directory for which profiling should be enabled.
> > >
> > > I've been digging through the git history, and I would swear I
> > > hallucinated the whole thing except I have the code in ktest for driving
> > > gcov and I swear it used to work :)
> > >
> > > Anyways - any thoughts on how we might implement this? I really need a
> > > way to specify directories to enable gcov for _without_ monkey patching;
> > > that's not a viable workflow in an automated setup.
> > >
> > > It seems like if we can get a list of directory prefixes for a path
> > > (e.g. given fs/bcachefs/btree_iter.o it would return fs, fs/bcachefs) it
> > > should be possible to extend the code you referenced.
> >
> > I'll likely not be able to implement this myself, but if you or anyone
> > else wants to go that route, here are my thoughts: $(src) should have
> > the relative source code path that is needed, so here's what needs to be
> > done:
> >
> > 1. Determine how to handle non-letter/digit/underscore characters in the
> > variable name:
> >
> > a) GCOV_PROFILE_fs/bcachefs => supported by GNU make [1], though
> > discouraged due to possible side-effects
> > b) GCOV_PROFILE_fs_bcachefs => might cause overlays, e.g. a/b/c and
> > a/b_c both have the same a_b_c suffix
> >
> > Personally I'd prefer option b)
> >
> > 2. Define a new Makefile variable that contains $(src) with required
> > character replacements (scripts/Kbuild.include might be a good place)
> >
> > 3. Add $(GCOV_PROFILE_$(name_of_that_new_var)) to the code quoted above
> > (scripts/Makefile.lib)
> >
> > 4. Document the use of this new Makefile variable in kernel/gcov/Kconfig
> > and Documentation/dev-tools/gcov.rst
> >
> > Since this new path-suffix version would be the first
> > GCOV_PROFILE-variable that is primarily intended to be specified on the
> > make command line and not added to a Makefile, it should probably take
> > precedence over all other versions. To achieve that it would need to be
> > specified first in the $(patsubst) statement.
> >
> > Once implemented, one might also consider extending this new support to
> > other variables like KASAN_SANITIZE or KCOV_INSTRUMENT, since all of
> > these are implemented the same way. I don't know whether that's useful
> > in all cases though.
> >
> > [1] https://www.gnu.org/software/make/manual/make.html#Using-Variables
> >
> > --
> > Peter Oberparleiter
> > Linux on IBM Z Development - IBM Germany R&D
> >
>
>
> I do not think it is a sensible proposal.
>
> Another idea could be something like
> CONFIG_GCOV_PROFILE_PATH=fs/bcachefs
With that it would only be possible to enable profiling for a single
subdirectory, and that's not going to be enough; when doing code
coverage analysis of a module we may want to profile library code/other
modules it depends on.
> or isn't it possible to filter dynamically?
> I think ftrace can change filtering dynamically.
> I do not know if it is possible for GCOV because I do not use it.
gcov requires different build flags, so no, not possible.
next prev parent reply other threads:[~2023-06-15 14:22 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-09 22:23 Specifying CFLAGS for a directory on the command line Kent Overstreet
2023-06-09 23:11 ` Nick Desaulniers
2023-06-09 23:12 ` Nick Desaulniers
2023-06-09 23:36 ` Kent Overstreet
2023-06-12 16:18 ` Peter Oberparleiter
2023-06-13 18:22 ` Kent Overstreet
2023-06-15 9:54 ` Peter Oberparleiter
2023-06-15 10:39 ` Masahiro Yamada
2023-06-15 14:22 ` Kent Overstreet [this message]
2023-06-16 16:45 ` Peter Oberparleiter
2023-06-16 17:00 ` Kent Overstreet
2023-06-16 6:37 ` Kent Overstreet
2023-06-16 18:10 ` Peter Oberparleiter
2023-06-17 0:28 ` Kent Overstreet
2023-06-21 8:54 ` Peter Oberparleiter
2023-06-22 10:35 ` Kent Overstreet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZIsek6ChkwkTHf2+@moria.home.lan \
--to=kent.overstreet@linux.dev \
--cc=linux-kbuild@vger.kernel.org \
--cc=masahiroy@kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=oberpar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox