Re: [PATCHv11 6/6] asm-generic/io: Add logging support for MMIO accessors

[Arnd Bergmann]

On Thu, Apr 28, 2022 at 9:35 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Apr 28, 2022 at 12:59:13PM +0530, Sai Prakash Ranjan wrote:
> > On 4/28/2022 11:21 AM, Greg KH wrote:
> > > On Thu, Apr 28, 2022 at 09:00:13AM +0530, Sai Prakash Ranjan wrote:
>
> > -D__NO_FORTIFY, -D__DISABLE_EXPORTS, -DDISABLE_BRANCH_PROFILING".
>
> Those are compiler flags that affect gcc, not kernel code functionality.

It's normal for invasive instrumentation to need flags to disable them. If you
look at mm/kasan/Makefile, you see

KASAN_SANITIZE := n
UBSAN_SANITIZE := n
KCOV_INSTRUMENT := n
CC_FLAGS_KASAN_RUNTIME += -DDISABLE_BRANCH_PROFILING
CFLAGS_REMOVE_common.o = $(CC_FLAGS_FTRACE)

all of which disable one of the instrumentation options, either per file
or per directory, in order to break recursion.

I don't know where exactly there is a problem with recursion in
the MMIO trace, but I can imagine that one of the other instrumentations
ends up doing an MMIO operation on some machine.

The part you obviously need to avoid is that running 'perf record'
to trace the MMIOs itself triggers more MMIO.

> > > Also, I see that this "disable the trace" feature has already been asked
> > > for for 2 other drivers in the Android kernel tree, why not include
> > > those changes here as well?  That kind of shows that this new feature is
> > > limited in that driver authors are already wanting it disabled, even
> > > before it is accepted.
> >
> > That can be done later on top of this series right? This series mainly deals with adding
> > initial support for such tracing, there could be numerous drivers who might or might
> > not want the feature which can be added onto later. We can't actually identify all
> > the driver requirements upfront. As an example, we have already used the flag to
> > disable tracing for nVHE KVM, so we know how to use the flag.
>
> Again, make it explicit in the driver file itself that it is doing this,
> not in the Makefile, and I will not have any objections.

We discussed a few options already, but we can revisit this again. The
current solution has a per-file compile-time switch and a global runtime
switch for the tracepoint.

I think moving the tracepoint into the header would make it a per-file
runtime switch (depending on how it gets inlined etc). I think that would
avoid the need for having the compile-time disable switch, but may result
in an excessive number of tracepoints.

       Arnd