Sync tools headers with the kernel source

[PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Namhyung Kim 1 year, 10 months ago

Hello,

I'm gonna carry these changes on the perf tools tree.  I'll update the
vhost.h once it lands on the mainline.

This is the full explanation from Arnaldo:

There used to be no copies, with tools/ code using kernel headers
directly. From time to time tools/perf/ broke due to legitimate kernel
hacking. At some point Linus complained about such direct usage. Then we
adopted the current model.

The way these headers are used in perf are not restricted to just
including them to compile something.

There are sometimes used in scripts that convert defines into string
tables, etc, so some change may break one of these scripts, or new MSRs
may use some different #define pattern, etc.

E.g.:

  $ ls -1 tools/perf/trace/beauty/*.sh | head -5
  tools/perf/trace/beauty/arch_errno_names.sh
  tools/perf/trace/beauty/drm_ioctl.sh
  tools/perf/trace/beauty/fadvise.sh
  tools/perf/trace/beauty/fsconfig.sh
  tools/perf/trace/beauty/fsmount.sh
  $
  $ tools/perf/trace/beauty/fadvise.sh
  static const char *fadvise_advices[] = {
        [0] = "NORMAL",
        [1] = "RANDOM",
        [2] = "SEQUENTIAL",
        [3] = "WILLNEED",
        [4] = "DONTNEED",
        [5] = "NOREUSE",
  };
  $

The tools/perf/check-headers.sh script, part of the tools/ build
process, points out changes in the original files.

So its important not to touch the copies in tools/ when doing changes in
the original kernel headers, that will be done later, when
check-headers.sh inform about the change to the perf tools hackers.


Thanks,
Namhyung


Namhyung Kim (9):
  tools/include: Sync uapi/drm/i915_drm.h with the kernel sources
  tools/include: Sync uapi/linux/fs.h with the kernel sources
  tools/include: Sync uapi/linux/kvm.h and asm/kvm.h with the kernel
    sources
  tools/include: Sync uapi/sound/asound.h with the kernel sources
  tools/include: Sync x86 CPU feature headers with the kernel sources
  tools/include: Sync x86 asm/irq_vectors.h with the kernel sources
  tools/include: Sync x86 asm/msr-index.h with the kernel sources
  tools/include: Sync asm-generic/bitops/fls.h with the kernel sources
  tools/include: Sync arm64 asm/cputype.h with the kernel sources

 tools/arch/arm64/include/asm/cputype.h        |   4 +
 tools/arch/arm64/include/uapi/asm/kvm.h       |  15 +-
 tools/arch/powerpc/include/uapi/asm/kvm.h     |  45 +-
 tools/arch/s390/include/uapi/asm/kvm.h        | 315 +++++++-
 tools/arch/x86/include/asm/cpufeatures.h      |  17 +-
 .../arch/x86/include/asm/disabled-features.h  |  11 +-
 tools/arch/x86/include/asm/irq_vectors.h      |   2 -
 tools/arch/x86/include/asm/msr-index.h        |  74 +-
 .../arch/x86/include/asm/required-features.h  |   3 +-
 tools/arch/x86/include/uapi/asm/kvm.h         | 308 +++++++-
 tools/include/asm-generic/bitops/__fls.h      |   8 +-
 tools/include/asm-generic/bitops/fls.h        |   8 +-
 tools/include/uapi/drm/i915_drm.h             |  16 +
 tools/include/uapi/linux/fs.h                 |  30 +-
 tools/include/uapi/linux/kvm.h                | 689 +-----------------
 tools/include/uapi/sound/asound.h             |   4 +-
 16 files changed, 809 insertions(+), 740 deletions(-)


base-commit: 25e973a0e077da585e472b4cadb1d2f4c1113285
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Geert Uytterhoeven 1 year, 10 months ago

Hi Namhyung, Arnaldo,

On Mon, Apr 8, 2024 at 8:55 PM Namhyung Kim <namhyung@kernel.org> wrote:
> I'm gonna carry these changes on the perf tools tree.  I'll update the
> vhost.h once it lands on the mainline.
>
> This is the full explanation from Arnaldo:
>
> There used to be no copies, with tools/ code using kernel headers
> directly. From time to time tools/perf/ broke due to legitimate kernel
> hacking. At some point Linus complained about such direct usage. Then we
> adopted the current model.

Do you have a reference to that?
I was gonna reply to [PATCH 8/9] (where I was CCed) with "why can't
you use the original include file directly?", then read the cover letter.

Thanks!

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Arnaldo Carvalho de Melo 1 year, 10 months ago

On Tue, Apr 09, 2024 at 09:15:02AM +0200, Geert Uytterhoeven wrote:
> Hi Namhyung, Arnaldo,
> 
> On Mon, Apr 8, 2024 at 8:55 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > I'm gonna carry these changes on the perf tools tree.  I'll update the
> > vhost.h once it lands on the mainline.
> >
> > This is the full explanation from Arnaldo:
> >
> > There used to be no copies, with tools/ code using kernel headers
> > directly. From time to time tools/perf/ broke due to legitimate kernel
> > hacking. At some point Linus complained about such direct usage. Then we
> > adopted the current model.
> 
> Do you have a reference to that?

I would have to do a search on the mailing list to find the exact
exchange, this is from memory and from people asking this repeatedly.

Ingo replied with reasons similar to the ones I use in this canned
explanation when sending the notice of updating the files.

- Arnaldo

> I was gonna reply to [PATCH 8/9] (where I was CCed) with "why can't
> you use the original include file directly?", then read the cover letter.
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Namhyung Kim 1 year, 10 months ago

Hello,

On Tue, Apr 9, 2024 at 6:47 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Tue, Apr 09, 2024 at 09:15:02AM +0200, Geert Uytterhoeven wrote:
> > Hi Namhyung, Arnaldo,
> >
> > On Mon, Apr 8, 2024 at 8:55 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > > I'm gonna carry these changes on the perf tools tree.  I'll update the
> > > vhost.h once it lands on the mainline.
> > >
> > > This is the full explanation from Arnaldo:
> > >
> > > There used to be no copies, with tools/ code using kernel headers
> > > directly. From time to time tools/perf/ broke due to legitimate kernel
> > > hacking. At some point Linus complained about such direct usage. Then we
> > > adopted the current model.
> >
> > Do you have a reference to that?
>
> I would have to do a search on the mailing list to find the exact
> exchange, this is from memory and from people asking this repeatedly.
>
> Ingo replied with reasons similar to the ones I use in this canned
> explanation when sending the notice of updating the files.

This is the Ingo's reply:

https://lore.kernel.org/r/ZhTwPLliHXKPAJUQ@gmail.com/

Thanks,
Namhyung

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Arnaldo Carvalho de Melo 1 year, 10 months ago

On Mon, Apr 08, 2024 at 11:55:11AM -0700, Namhyung Kim wrote:
> Hello,
> 
> I'm gonna carry these changes on the perf tools tree.  I'll update the
> vhost.h once it lands on the mainline.

Humm, maybe its not a good idea to do that this cycle? I mean some of
these were moved from tools/{include,arch} to tools/perf/trace/beauty/{include,arch},
like tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h.

But having said that, probably isn't a problem, when you get this pulled
upstream I'll do a merge with perf-tools-next and this should be
trivially resolved, I think.

Using the above example:

⬢[acme@toolbox perf-tools-next]$ git log tools/perf/trace/beauty/arch/x86/include/asm/irq_vectors.h
commit c8bfe3fad4f86a029da7157bae9699c816f0c309
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Mon Mar 11 17:07:33 2024 -0300

    perf beauty: Move arch/x86/include/asm/irq_vectors.h copy out of the directory used to build perf
    
    It is used only to generate string tables, not to build perf, so move it
    to the tools/perf/trace/beauty/include/ hierarchy, that is used just for
    scraping.
    
    This is a something that should've have happened, as happened with the
    linux/socket.h scrapper, do it now as Ian suggested while doing an
    audit/refactor session in the headers used by perf.
    
    No other tools/ living code uses it.
    
    Suggested-by: Ian Rogers <irogers@google.com>
    Reviewed-by: Ian Rogers <irogers@google.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Link: https://lore.kernel.org/lkml/CAP-5=fWZVrpRufO4w-S4EcSi9STXcTAN2ERLwTSN7yrSSA-otQ@mail.gmail.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo
 
> This is the full explanation from Arnaldo:
> 
> There used to be no copies, with tools/ code using kernel headers
> directly. From time to time tools/perf/ broke due to legitimate kernel
> hacking. At some point Linus complained about such direct usage. Then we
> adopted the current model.
> 
> The way these headers are used in perf are not restricted to just
> including them to compile something.
> 
> There are sometimes used in scripts that convert defines into string
> tables, etc, so some change may break one of these scripts, or new MSRs
> may use some different #define pattern, etc.
> 
> E.g.:
> 
>   $ ls -1 tools/perf/trace/beauty/*.sh | head -5
>   tools/perf/trace/beauty/arch_errno_names.sh
>   tools/perf/trace/beauty/drm_ioctl.sh
>   tools/perf/trace/beauty/fadvise.sh
>   tools/perf/trace/beauty/fsconfig.sh
>   tools/perf/trace/beauty/fsmount.sh
>   $
>   $ tools/perf/trace/beauty/fadvise.sh
>   static const char *fadvise_advices[] = {
>         [0] = "NORMAL",
>         [1] = "RANDOM",
>         [2] = "SEQUENTIAL",
>         [3] = "WILLNEED",
>         [4] = "DONTNEED",
>         [5] = "NOREUSE",
>   };
>   $
> 
> The tools/perf/check-headers.sh script, part of the tools/ build
> process, points out changes in the original files.
> 
> So its important not to touch the copies in tools/ when doing changes in
> the original kernel headers, that will be done later, when
> check-headers.sh inform about the change to the perf tools hackers.
> 
> 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (9):
>   tools/include: Sync uapi/drm/i915_drm.h with the kernel sources
>   tools/include: Sync uapi/linux/fs.h with the kernel sources
>   tools/include: Sync uapi/linux/kvm.h and asm/kvm.h with the kernel
>     sources
>   tools/include: Sync uapi/sound/asound.h with the kernel sources
>   tools/include: Sync x86 CPU feature headers with the kernel sources
>   tools/include: Sync x86 asm/irq_vectors.h with the kernel sources
>   tools/include: Sync x86 asm/msr-index.h with the kernel sources
>   tools/include: Sync asm-generic/bitops/fls.h with the kernel sources
>   tools/include: Sync arm64 asm/cputype.h with the kernel sources
> 
>  tools/arch/arm64/include/asm/cputype.h        |   4 +
>  tools/arch/arm64/include/uapi/asm/kvm.h       |  15 +-
>  tools/arch/powerpc/include/uapi/asm/kvm.h     |  45 +-
>  tools/arch/s390/include/uapi/asm/kvm.h        | 315 +++++++-
>  tools/arch/x86/include/asm/cpufeatures.h      |  17 +-
>  .../arch/x86/include/asm/disabled-features.h  |  11 +-
>  tools/arch/x86/include/asm/irq_vectors.h      |   2 -
>  tools/arch/x86/include/asm/msr-index.h        |  74 +-
>  .../arch/x86/include/asm/required-features.h  |   3 +-
>  tools/arch/x86/include/uapi/asm/kvm.h         | 308 +++++++-
>  tools/include/asm-generic/bitops/__fls.h      |   8 +-
>  tools/include/asm-generic/bitops/fls.h        |   8 +-
>  tools/include/uapi/drm/i915_drm.h             |  16 +
>  tools/include/uapi/linux/fs.h                 |  30 +-
>  tools/include/uapi/linux/kvm.h                | 689 +-----------------
>  tools/include/uapi/sound/asound.h             |   4 +-
>  16 files changed, 809 insertions(+), 740 deletions(-)
> 
> 
> base-commit: 25e973a0e077da585e472b4cadb1d2f4c1113285
> -- 
> 2.44.0.478.gd926399ef9-goog

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Ingo Molnar 1 year, 10 months ago

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> On Mon, Apr 08, 2024 at 11:55:11AM -0700, Namhyung Kim wrote:
> > Hello,
> > 
> > I'm gonna carry these changes on the perf tools tree.  I'll update the
> > vhost.h once it lands on the mainline.
> 
> Humm, maybe its not a good idea to do that this cycle?

Maybe it's just me, but I've been looking sadly at all the header warnings 
for months. :-) Would be better to keep them in sync with a bit higher 
frequency, IMO - which would reduce the pain and churn rate:

> >  16 files changed, 809 insertions(+), 740 deletions(-)

That's like about a year of changes missed? An update once per cycle and 
this wouldn't be nearly as painful, right?

Thanks,

	Ingo

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Arnaldo Carvalho de Melo 1 year, 10 months ago

On Tue, Apr 09, 2024 at 09:40:29AM +0200, Ingo Molnar wrote:
> 
> * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > On Mon, Apr 08, 2024 at 11:55:11AM -0700, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > I'm gonna carry these changes on the perf tools tree.  I'll update the
> > > vhost.h once it lands on the mainline.
> > 
> > Humm, maybe its not a good idea to do that this cycle?
> 
> Maybe it's just me, but I've been looking sadly at all the header warnings 
> for months. :-) Would be better to keep them in sync with a bit higher 

I backtracked from asking for a delay, the reason for me first thinking
about not doing it this time is because in perf-tools-next we have, to
be merged for 6.10:

⬢[acme@toolbox perf-tools-next]$ git log --oneline torvalds/master.. | tail
f324b73c2c05832b perf beauty: Stop using the copy of uapi/linux/prctl.h
c8bfe3fad4f86a02 perf beauty: Move arch/x86/include/asm/irq_vectors.h copy out of the directory used to build perf
7050e33e86ad03d2 perf beauty: Move uapi/sound/asound.h copy out of the directory used to build perf
44512bd6136ec7bb perf beauty: Move uapi/linux/usbdevice_fs.h copy out of the directory used to build perf
ab3316119f9d0b3a perf beauty: Move uapi/linux/mount.h copy out of the directory used to build perf
22916d2cbad9a20d perf beauty: Don't include uapi/linux/mount.h, use sys/mount.h instead
faf7217a397f041f perf beauty: Move uapi/linux/fs.h copy out of the directory used to build perf
5d8c646038f2f173 perf beauty: Fix dependency of tables using uapi/linux/mount.h
4b3761eebb1c5c1b perf c2c: Fix a punctuation
a9f4c6c999008c92 perf trace: Collect sys_nanosleep first argument
⬢[acme@toolbox perf-tools-next]$

The reasoning for these changes is in the csets, for instance:

⬢[acme@toolbox perf-tools-next]$ git show 7050e33e86ad03d2 | head -21
commit 7050e33e86ad03d26d7b969bba1d48ee159be496
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Mon Mar 11 17:07:33 2024 -0300

    perf beauty: Move uapi/sound/asound.h copy out of the directory used to build perf

    It is used only to generate string tables, not to build perf, so move it
    to the tools/perf/trace/beauty/include/ hierarchy, that is used just for
    scraping.

    This is a something that should've have happened, as happened with the
    linux/socket.h scraper, do it now as Ian suggested while doing an
    audit/refactor session in the headers used by perf.

    Suggested-by: Ian Rogers <irogers@google.com>
    Reviewed-by: Ian Rogers <irogers@google.com>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Link: https://lore.kernel.org/lkml/CAP-5=fWZVrpRufO4w-S4EcSi9STXcTAN2ERLwTSN7yrSSA-otQ@mail.gmail.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
⬢[acme@toolbox perf-tools-next]$

I.e. I had moved some of the stuff that is being updated, but nah,
better to have v6.9 tools/perf building without warnings, I'll fixup the
merge when I merge perf-tools-next with torvalds/master after the
updates are merged upstream.

This is sometimes difficult to achieve tho or would require more pull
requests to be sent to Linus to get some last minute changes, its not
something common, but I think happened a few times.

> frequency, IMO - which would reduce the pain and churn rate:
> 
> > >  16 files changed, 809 insertions(+), 740 deletions(-)
> 
> That's like about a year of changes missed? An update once per cycle and 
> this wouldn't be nearly as painful, right?

Humm, I haven't checked if it stays that long without merging, what I've
agreed with Namhyung is to wait a bit for things to reach upstream, say
-rc3, and then do the update, trying not to do these things nor early
not late in the -rc sequence, to pick a sweet spot where most likely no
changes will be made, we make the update and the perf build has no
warnings in the final release.

- Arnaldo

Re: [PATCHSET 0/9] Sync tools headers with the kernel source

Posted by Namhyung Kim 1 year, 10 months ago

Hi Arnaldo and Ingo,

On Tue, Apr 9, 2024 at 6:58 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Tue, Apr 09, 2024 at 09:40:29AM +0200, Ingo Molnar wrote:
> >
> > * Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >
> > > On Mon, Apr 08, 2024 at 11:55:11AM -0700, Namhyung Kim wrote:
> > > > Hello,
> > > >
> > > > I'm gonna carry these changes on the perf tools tree.  I'll update the
> > > > vhost.h once it lands on the mainline.
> > >
> > > Humm, maybe its not a good idea to do that this cycle?
> >
> > Maybe it's just me, but I've been looking sadly at all the header warnings
> > for months. :-) Would be better to keep them in sync with a bit higher
>
> I backtracked from asking for a delay, the reason for me first thinking
> about not doing it this time is because in perf-tools-next we have, to
> be merged for 6.10:
>
> ⬢[acme@toolbox perf-tools-next]$ git log --oneline torvalds/master.. | tail
> f324b73c2c05832b perf beauty: Stop using the copy of uapi/linux/prctl.h
> c8bfe3fad4f86a02 perf beauty: Move arch/x86/include/asm/irq_vectors.h copy out of the directory used to build perf
> 7050e33e86ad03d2 perf beauty: Move uapi/sound/asound.h copy out of the directory used to build perf
> 44512bd6136ec7bb perf beauty: Move uapi/linux/usbdevice_fs.h copy out of the directory used to build perf
> ab3316119f9d0b3a perf beauty: Move uapi/linux/mount.h copy out of the directory used to build perf
> 22916d2cbad9a20d perf beauty: Don't include uapi/linux/mount.h, use sys/mount.h instead
> faf7217a397f041f perf beauty: Move uapi/linux/fs.h copy out of the directory used to build perf
> 5d8c646038f2f173 perf beauty: Fix dependency of tables using uapi/linux/mount.h
> 4b3761eebb1c5c1b perf c2c: Fix a punctuation
> a9f4c6c999008c92 perf trace: Collect sys_nanosleep first argument
> ⬢[acme@toolbox perf-tools-next]$
>
> The reasoning for these changes is in the csets, for instance:
>
> ⬢[acme@toolbox perf-tools-next]$ git show 7050e33e86ad03d2 | head -21
> commit 7050e33e86ad03d26d7b969bba1d48ee159be496
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Mon Mar 11 17:07:33 2024 -0300
>
>     perf beauty: Move uapi/sound/asound.h copy out of the directory used to build perf
>
>     It is used only to generate string tables, not to build perf, so move it
>     to the tools/perf/trace/beauty/include/ hierarchy, that is used just for
>     scraping.
>
>     This is a something that should've have happened, as happened with the
>     linux/socket.h scraper, do it now as Ian suggested while doing an
>     audit/refactor session in the headers used by perf.
>
>     Suggested-by: Ian Rogers <irogers@google.com>
>     Reviewed-by: Ian Rogers <irogers@google.com>
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Link: https://lore.kernel.org/lkml/CAP-5=fWZVrpRufO4w-S4EcSi9STXcTAN2ERLwTSN7yrSSA-otQ@mail.gmail.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ⬢[acme@toolbox perf-tools-next]$
>
> I.e. I had moved some of the stuff that is being updated, but nah,
> better to have v6.9 tools/perf building without warnings, I'll fixup the
> merge when I merge perf-tools-next with torvalds/master after the
> updates are merged upstream.

Yeah I noticed that, but I thought it's better to keep v6.9 headers in
sync.  Please take care of them in perf-tools-next for v6.10.

>
> This is sometimes difficult to achieve tho or would require more pull
> requests to be sent to Linus to get some last minute changes, its not
> something common, but I think happened a few times.
>
> > frequency, IMO - which would reduce the pain and churn rate:
> >
> > > >  16 files changed, 809 insertions(+), 740 deletions(-)
> >
> > That's like about a year of changes missed? An update once per cycle and
> > this wouldn't be nearly as painful, right?
>
> Humm, I haven't checked if it stays that long without merging, what I've
> agreed with Namhyung is to wait a bit for things to reach upstream, say
> -rc3, and then do the update, trying not to do these things nor early
> not late in the -rc sequence, to pick a sweet spot where most likely no
> changes will be made, we make the update and the perf build has no
> warnings in the final release.

I think we're doing it once per release cycle already.  This is just for v6.9.
The kvm changes moved some code to arch directories.

This is what I did for v6.7.

https://lore.kernel.org/r/20231121225650.390246-1-namhyung@kernel.org/

Thanks,
Namhyung