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: [PATCH] gcov,x86: Mark GCOV broken for x86
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org,
        johannes.berg@intel.com, ndesaulniers@google.com,
        nathan@kernel.org, keescook@chromium.org, elver@google.com,
        mark.rutland@arm.com
References: <YMcssV/n5IBGv4f0@hirez.programming.kicks-ass.net>
 <2f8a4e21-a09b-8c8d-54ce-45cf2f0e83ff@linux.ibm.com>
 <YMx/9Xv8BF7ghAO6@hirez.programming.kicks-ass.net>
From:   Peter Oberparleiter <oberpar@linux.ibm.com>
Message-ID: <0df5ec31-46ac-4f50-26f5-c761371198c9@linux.ibm.com>
Date:   Mon, 21 Jun 2021 15:53:48 +0200
In-Reply-To: <YMx/9Xv8BF7ghAO6@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

On 18.06.2021 13:13, Peter Zijlstra wrote:
> On Mon, Jun 14, 2021 at 04:43:27PM +0200, Peter Oberparleiter wrote:
>> On 14.06.2021 12:17, Peter Zijlstra wrote:
>> If there was a way to automatically identify 'noinstr'-afflicted source
>> files (e.g. by grepping the pre-processed source files), one could also
>> automate this process by adjusting the kbuild-code that adds profiling
>> flags to automatically exclude such files.
> 
> Or we just wait for the compilers to implement the required function
> attribute and then make the whole thing depend on having a recent enough
> compiler, which is what I'm hoping for.

Sounds like the best approach given the current situation.

> Developers should use recent compilers anyway...
> 
>>> Until such time as that compilers have added a function attribute to
>>> disable this instrumentation, mark GCOV as broken.
>>>
>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[...]

>> Users that are absolutely sure that their code can work with
>> gcov-profiling can manually edit their sub-Makefiles to list those files
>> that should be instrumented. In my opinion your introduction of
>> ARCH_HAS_GCOV_BROKEN unnecessarily takes away this capability.
> 
> Are there any users?

I honestly cannot tell how many people are using the gcov-kernel
facility, but I guess that is true for most kernel functions. I do get
regular bug reports if things break due to GCC changes though, so there
are definitely some users.

> Who uses this GCOV stuff, and should we migrate them to KCOV?

I have not used KCOV myself, but based on the documentation available
about the mechanism itself and some of its users it appears that GCOV
and KCOV are not feature-equivalent:

KCOV provides an opt-in capability for user-space processes to record a
history of kernel code executed on their behalf. Opt-in means the
process must include code to start recording. There's also some limited
support for collecting data for kernel background tasks.

The output is a sequence of kernel addresses relating to executed basic
blocks - great for consumption in automated mechanisms like fuzzers and
the like, but tooling for post-processing the data into human-readable
format for general purpose review seems to be scarce.

GCOV provides a build-time configurable option to instrument almost all
kernel code or, if requested, only specific parts. There's no need to
opt-in at run-time, all execution of instrumented code running since
boot will be recorded, including interrupt handlers, background tasks,
etc. Of course this also means there's no means to filter by the process
causing the kernel code execution.

The output is GCC's .gcda data format. Using GCC's gcov tool this can be
directly converted into annotated source code containing statement,
function, and branch coverage data. Also there are tools building on
gcov output to create overview pages for the thousands of kernel source
files and detailed graphical output based on this information. This kind
of representation is great for manual review to answer questions like
"all my tests ran, why is there still an uncovered piece of kernel code?"

So to summarize:

KCOV => Great for automated processing (e.g. fuzzing)

GCOV => Great for manual review (e.g. improving test coverage)

> The thing is, I got dead kernel reports from KCOV users really quickly
> after all this landed, I've never even heard of a GCOV user, let alone
> had a problem report from one.

KCOV is used 24/7 by automated fuzzing code - I would assume that those
catch fatal errors rather quickly. GCOV is used when a developer/tester
has a need to review their test coverage. And even then they may only
instrument portions of the kernel that may not affected by the noinstr
problem.

> Given all this seems mostly unused, I suppose we can wait for the
> compilers to implement the attribute and simply ignore any and all
> problems stemming from the use of GCOV -- telling them to go use KCOV
> instead.

It seems this might be resolved for GCC rather quickly...

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223#c11

> At the same time; since there are no users (that I know of), I don't see
> the problem with killing the entire thing for x86 either.

If there really were no more GCOV users and KCOV would provide the same
functionality and level of tool support I would not object. But IMO none
of these requirements are met today.


Regards,
  Peter

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany