On 15.06.2023 12:39, Masahiro Yamada wrote:
> On Thu, Jun 15, 2023 at 6:54 PM Peter Oberparleiter
> <oberpar@xxxxxxxxxxxxx> 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
>>
[...]
> I do not think it is a sensible proposal.
Does your objection apply to the proposed implementation, or the feature
request itself? If the former, what aspect of the approach do you
consider problematic?
> Another idea could be something like
> CONFIG_GCOV_PROFILE_PATH=fs/bcachefs
> or isn't it possible to filter dynamically?
Using CONFIG symbols would be another way to specify the input list of
directories that should be instrumented for GCOV profiling. The output
would still need to be a different c_flags value for source files in
these directories though. I would assume that getting this information
from CONFIG values into c_flags would be a more intrusive change
compared to the route via a Makefile variable.
> I think ftrace can change filtering dynamically.
> I do not know if it is possible for GCOV because I do not use it.
GCOV instrumentation is a per-source file build-time decision. There's
no way to dynamically turn it on or off during run-time.
> But, the best would be to not implement it at all
> until we know that it is a wide demand.
> The upstream is not a place to cater to every feature request.
I don't feel very strongly about this specific feature - it seemed to me
that it could be implemented in a non-intrusive way and it provides
value to at least one developer.
--
Peter Oberparleiter
Linux on IBM Z Development - IBM Germany R&D
