Re: [PATCH] perf kvm stat: Fix build error

[Ian Rogers]

On Thu, Feb 26, 2026 at 11:32 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> On Thu, Feb 26, 2026 at 08:44:51AM -0800, Ian Rogers wrote:
> > On Thu, Feb 26, 2026 at 1:52 AM Leo Yan <leo.yan@xxxxxxx> wrote:
> > >
> > > On Wed, Feb 25, 2026 at 10:36:26PM -0800, Ian Rogers wrote:
> > >
> > > [...]
> > >
> > > > Hmm.. it looks like something has gone screwy with the include paths.
> > > > The header you've gotten is:
> > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/arch/x86/include/uapi/asm/svm.h#n137
> > > > rather than:
> > > > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/arch/x86/include/uapi/asm/svm.h#n137
> > > > where the value differs between -1ull and -1.
> > > >
> > > > In Leo's change he has the include path as:
> > > >   #include "../../arch/x86/include/uapi/asm/svm.h"
> > > >
> > > > but I think that is off by one and should be:
> > > >   #include "../../../arch/x86/include/uapi/asm/svm.h"
> > >
> > > Sorry I did not check the header path carefully.  A bit thoughts:
> > >
> > > The perf build will not include headers in relative paths as we
> > > expected.
> > >
> > > It tries to find headers with appending search paths.  As Ian said,
> > > I saw .kvm-stat-x86.o.cmd that includes kernel headers:
> > >
> > >   /home/niayan01/Work/linux/tools/include/../../arch/x86/include/uapi/asm/svm.h \
> > >   /home/niayan01/Work/linux/tools/include/../../arch/x86/include/uapi/asm/vmx.h \
> > >   /home/niayan01/Work/linux/tools/include/../../arch/x86/include/uapi/asm/kvm.h \
> > >
> > > I'd suggest a fix in Makefile as below.  The idea is to give priority to
> > > the lower level folders under perf, so that headers in the deeper perf
> > > directories are searched first.  This avoids searching any kernel headers.
>
> It fixes the problem, thanks!
>
> Tested-by: Namhyung Kim <namhyung@xxxxxxxxxx>

So it is still the wrong fix. The relative include paths is the real
issue, just add the missing "../". I'm not a fan of changing the
include order as it has the potential to break things due to the
subtle way that includes are picked up, tools/include isn't hermetic,
etc. While it may fix one build failure we are likely creating build
failures for others - well me, with bazel, for sure.

Thanks,
Ian


> More comments below.
>
> >
> > I'm not opposed to the change but I still think we should make the
> > relative include be to the correct files. Some other thoughts below.
> >
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index a8dc72cfe48e..47359f672b6a 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -388,8 +388,10 @@ ifeq ($(DEBUG),0)
> > >    endif
> > >  endif
> > >
> > > -INC_FLAGS += -I$(src-perf)/util/include
> >
> > Not clear to me why we need a util/include on top of everything else.
> > It seems somewhat random what is in util/include rather than just in
> > util.
>
> It seems it tries to override some kernel headers but I'm curious we can
> move the files to the tools/include and get rid of the directory.
>
> >
> > >  INC_FLAGS += -I$(src-perf)/arch/$(SRCARCH)/include
> >
> > Hopefully with the removal of the arch directory (and fixing of
> > cross-platform profiling) in slow chunks by changes like:
> > https://lore.kernel.org/lkml/20260224142938.26088-1-irogers@xxxxxxxxxx/
> > we can get rid of this one.
> >
> > > +INC_FLAGS += -I$(src-perf)/util/include
> > > +INC_FLAGS += -I$(src-perf)/util
>
> I also hope to remove $(src-perf)/util too and use path relative to
> $(src-perf) always for perf-specific headers.  But this is a separate
> topic.
>
>
> > > +INC_FLAGS += -I$(src-perf)
> >
> > This:
> >
> > >  INC_FLAGS += -I$(srctree)/tools/include/
> > >  INC_FLAGS += -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi
> > >  INC_FLAGS += -I$(srctree)/tools/include/uapi
> >
> > to here means that we prefer linux headers over uapi headers, which I
> > think is the opposite of what a user-land process would expect. I
> > think we should move these headers into two libraries, build them with
> > an `install_headers` rule, and make the include path pick up the
> > headers from where they are installed. As the linux and uapi headers
> > are licensed differently, this would clarify the dependencies and
> > licensing implications.
>
> Sounds good.
>
> Thanks,
> Namhyung
>
> >
> > There's another can of worms regarding whether libperf is something
> > useful, outside of declarations in event.h.
> >
> > Thanks,
> > Ian
> >
> > > @@ -403,9 +405,6 @@ INC_FLAGS += -I$(obj-perf)/util
> > >  INC_FLAGS += -I$(obj-perf)
> > >  endif
> > >
> > > -INC_FLAGS += -I$(src-perf)/util
> > > -INC_FLAGS += -I$(src-perf)
> > > -
> > >  CORE_CFLAGS += -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_
> > >
> > > As a result, I can get the search paths in .kvm-stat-x86.o.cmd:
> > >
> > >   /home/niayan01/Work/linux/tools/perf/util/../../arch/x86/include/uapi/asm/svm.h \
> > >   /home/niayan01/Work/linux/tools/perf/util/../../arch/x86/include/uapi/asm/vmx.h \
> > >   /home/niayan01/Work/linux/tools/perf/util/../../arch/x86/include/uapi/asm/kvm.h \
> > >
> > > Please let me know if this makes sense.
> > >
> > > Thanks,
> > > Leo