Hello,

On Mon, May 31, 2021 at 10:35:54AM +0100, Mel Gorman wrote:

On Sat, May 29, 2021 at 08:10:36PM -0700, Andrii Nakryiko wrote:

quoted

On Fri, May 28, 2021 at 12:42 AM Mel Gorman [off-list ref] wrote:

quoted

On Thu, May 27, 2021 at 03:17:48PM -0700, Andrii Nakryiko wrote:

quoted

quoted

Andrii Nakryiko bisected the problem to the commit "mm/page_alloc: convert
per-cpu list protection to local_lock" currently staged in mmotm. In his
own words

  The immediate problem is two different definitions of numa_node per-cpu
  variable. They both are at the same offset within .data..percpu ELF
  section, they both have the same name, but one of them is marked as
  static and another as global. And one is int variable, while another
  is struct pagesets. I'll look some more tomorrow, but adding Jiri and
  Arnaldo for visibility.

  [110907] DATASEC '.data..percpu' size=178904 vlen=303
  ...
        type_id=27753 offset=163976 size=4 (VAR 'numa_node')
        type_id=27754 offset=163976 size=4 (VAR 'numa_node')

  [27753] VAR 'numa_node' type_id=27556, linkage=static
  [27754] VAR 'numa_node' type_id=20, linkage=global

  [20] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED

  [27556] STRUCT 'pagesets' size=0 vlen=1
        'lock' type_id=507 bits_offset=0

  [506] STRUCT '(anon)' size=0 vlen=0
  [507] TYPEDEF 'local_lock_t' type_id=506

The patch in question introduces a zero-sized per-cpu struct and while
this is not wrong, versions of pahole prior to 1.22 get confused during
BTF generation with two separate variables occupying the same address.

This patch adds a requirement for pahole 1.22 before setting
DEBUG_INFO_BTF.  While pahole 1.22 does not exist yet, a fix is in the
pahole git tree as ("btf_encoder: fix and complete filtering out zero-sized
per-CPU variables").

Reported-by: Michal Suchanek <redacted>
Reported-by: Hritik Vijay <redacted>
Debugged-by: Andrii Nakryiko [off-list ref]
Signed-off-by: Mel Gorman <redacted>
---

I still think that v1 ([0]) is a more appropriate temporary solution
until pahole 1.22 is released and widely packaged. Suddenly raising
the minimum version to 1.22, which is not even released even, is a
pretty big compatibility concern for all the users that rely on
CONFIG_DEBUG_INFO_BTF.

On the flip side, we have a situation where a build tool (pahole) has a
problem whereby correct code does not result in a working kernel. It's
not that dissimilar to preventing the kernel being built on an old
compiler. While I accept it's unfortunate, Christoph had a point where
introducing workarounds in the kernel could lead to a prolification of
workarounds for pahole or other reasons that are potentially tricky to
revert as long as distributions exist that do not ship with a sufficiently
reason package.

quoted

Just a few days ago pahole 1.16 worked fine and
here we suddenly (and silently due to how Kconfig functions) raise
that to a version that doesn't exist. That's going to break workflows
for a lot of people.

People do have a workaround though. For the system building the kernel,
they can patch pahole and revert the check so a bootable kernel can be
built. It's not convenient but it is manageable and pahole has until
5.13 releases to release a v1.22. The downsides for the alternative --
a non-booting kernel are much more severe.

quoted

I'm asking to have that ugly work-around to ensure sizeof(struct
pagesets) > 0 as a temporary solution only.

Another temporary solution is to locally build pahole and either revert
the check or fake the 1.22 release number with the self-built pahole.

Well, luckily it seems we anticipated issues like that and added
--skip_encoding_btf_vars argument, which I completely forgot about and
just accidentally came across reviewing Arnaldo's latest pahole patch.
I think that one is a much better solution, as then it will impact
only those that explicitly relies on availability of BTF for per-CPU
variables, which is a subset of all possible uses for kernel BTF. Sent
a patch ([0]), please take a look.

  [0] https://lore.kernel.org/linux-mm/20210530002536.3193829-1-andrii@kernel.org/T/#u (local)

I'm happy to have this patch used as an alternative to forcing 1.22 to
be the minimum version of pahole required.

Is pahole 1.22 available already?

Adding the a patch that reports different version is kind of annoying.

Thanks

Michal