| Message ID | 20210527171923.GG30378@techsingularity.net |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] mm/page_alloc: Require pahole v1.22 to cope with zero-sized struct pagesets | expand |
On Thu, May 27, 2021 at 10:19 AM Mel Gorman <mgorman@techsingularity.net> wrote: > > This patch replaces > mm-page_alloc-convert-per-cpu-list-protection-to-local_lock-fix.patch in > Andrew's tree. > > Michal Suchanek reported the following problem with linux-next > > [ 0.000000] Linux version 5.13.0-rc2-next-20210519-1.g3455ff8-vanilla (geeko@buildhost) (gcc (SUSE Linux) 10.3.0, GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.36.1.20210326-3) #1 SMP Wed May 19 10:05:10 UTC 2021 (3455ff8) > [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.13.0-rc2-next-20210519-1.g3455ff8-vanilla root=UUID=ec42c33e-a2c2-4c61-afcc-93e9527 8f687 plymouth.enable=0 resume=/dev/disk/by-uuid/f1fe4560-a801-4faf-a638-834c407027c7 mitigations=auto earlyprintk initcall_debug nomodeset earlycon ignore_loglevel console=ttyS0,115200 > ... > [ 26.093364] calling tracing_set_default_clock+0x0/0x62 @ 1 > [ 26.098937] initcall tracing_set_default_clock+0x0/0x62 returned 0 after 0 usecs > [ 26.106330] calling acpi_gpio_handle_deferred_request_irqs+0x0/0x7c @ 1 > [ 26.113033] initcall acpi_gpio_handle_deferred_request_irqs+0x0/0x7c returned 0 after 3 usecs > [ 26.121559] calling clk_disable_unused+0x0/0x102 @ 1 > [ 26.126620] initcall clk_disable_unused+0x0/0x102 returned 0 after 0 usecs > [ 26.133491] calling regulator_init_complete+0x0/0x25 @ 1 > [ 26.138890] initcall regulator_init_complete+0x0/0x25 returned 0 after 0 usecs > [ 26.147816] Freeing unused decrypted memory: 2036K > [ 26.153682] Freeing unused kernel image (initmem) memory: 2308K > [ 26.165776] Write protecting the kernel read-only data: 26624k > [ 26.173067] Freeing unused kernel image (text/rodata gap) memory: 2036K > [ 26.180416] Freeing unused kernel image (rodata/data gap) memory: 1184K > [ 26.187031] Run /init as init process > [ 26.190693] with arguments: > [ 26.193661] /init > [ 26.195933] with environment: > [ 26.199079] HOME=/ > [ 26.201444] TERM=linux > [ 26.204152] BOOT_IMAGE=/boot/vmlinuz-5.13.0-rc2-next-20210519-1.g3455ff8-vanilla > [ 26.254154] BPF: type_id=35503 offset=178440 size=4 > [ 26.259125] BPF: > [ 26.261054] BPF:Invalid offset > [ 26.264119] BPF: > [ 26.264119] > [ 26.267437] failed to validate module [efivarfs] BTF: -22 > > 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 <msuchanek@suse.de> > Reported-by: Hritik Vijay <hritikxx8@gmail.com> > Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > --- 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. 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. I'm asking to have that ugly work-around to ensure sizeof(struct pagesets) > 0 as a temporary solution only. If we have to raise the minimum pahole version to 1.22 we should also use that as an opportunity to clean up and simplify pahole integration in Kbuild. Kernel-side work-around will give us enough time to test and validate everything instead of rushing bug fix release. Once 1.22 is widely available we'll get rid of work-around and make scripts/link-vmlinux.sh simpler (see discussion in [1]). [0] https://lore.kernel.org/bpf/20210526080741.GW30378@techsingularity.net/ [1] https://lore.kernel.org/bpf/CAEf4BzaTP_jULKMN_hx6ZOqwESOmsR6_HxWW-LnrA5xwRNtSWg@mail.gmail.com/ > lib/Kconfig.debug | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index 678c13967580..825be101767e 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -301,10 +301,14 @@ config DEBUG_INFO_DWARF5 > > endchoice # "DWARF version" > > +config PAHOLE_HAS_ZEROSIZE_PERCPU_SUPPORT > + def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "122") > + > config DEBUG_INFO_BTF > bool "Generate BTF typeinfo" > depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED > depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST > + depends on PAHOLE_HAS_ZEROSIZE_PERCPU_SUPPORT > help > Generate deduplicated BTF type information from DWARF debug info. > Turning this on expects presence of pahole tool, which will convert
On Thu, May 27, 2021 at 03:17:48PM -0700, Andrii Nakryiko wrote: > > 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 <msuchanek@suse.de> > > Reported-by: Hritik Vijay <hritikxx8@gmail.com> > > Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > --- > > 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. > 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. > 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. -- Mel Gorman SUSE Labs
On Fri, May 28, 2021 at 12:42 AM Mel Gorman <mgorman@techsingularity.net> wrote: > > On Thu, May 27, 2021 at 03:17:48PM -0700, Andrii Nakryiko wrote: > > > 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 <msuchanek@suse.de> > > > Reported-by: Hritik Vijay <hritikxx8@gmail.com> > > > Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > > --- > > > > 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. > > > 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. > > > 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 > -- > Mel Gorman > SUSE Labs
On Sat, May 29, 2021 at 08:10:36PM -0700, Andrii Nakryiko wrote: > On Fri, May 28, 2021 at 12:42 AM Mel Gorman <mgorman@techsingularity.net> wrote: > > > > On Thu, May 27, 2021 at 03:17:48PM -0700, Andrii Nakryiko wrote: > > > > 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 <msuchanek@suse.de> > > > > Reported-by: Hritik Vijay <hritikxx8@gmail.com> > > > > Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> > > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > > > --- > > > > > > 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. > > > > > 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. > > > > > 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 I'm happy to have this patch used as an alternative to forcing 1.22 to be the minimum version of pahole required. -- Mel Gorman SUSE Labs
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: > > On Fri, May 28, 2021 at 12:42 AM Mel Gorman <mgorman@techsingularity.net> wrote: > > > > > > On Thu, May 27, 2021 at 03:17:48PM -0700, Andrii Nakryiko wrote: > > > > > 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 <msuchanek@suse.de> > > > > > Reported-by: Hritik Vijay <hritikxx8@gmail.com> > > > > > Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> > > > > > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > > > > > --- > > > > > > > > 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. > > > > > > > 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. > > > > > > > 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 > > 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
On Thu, Jul 15, 2021 at 09:44:53PM +0200, Michal Such?nek wrote: > > > 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 > > > > 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? > Ultimately it was of less importance because of a0b8200d06ad ("kbuild: skip per-CPU BTF generation for pahole v1.18-v1.21"). As I write this, pahole v1.22 has not been tagged. -- Mel Gorman SUSE Labs
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 678c13967580..825be101767e 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -301,10 +301,14 @@ config DEBUG_INFO_DWARF5 endchoice # "DWARF version" +config PAHOLE_HAS_ZEROSIZE_PERCPU_SUPPORT + def_bool $(success, test `$(PAHOLE) --version | sed -E 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "122") + config DEBUG_INFO_BTF bool "Generate BTF typeinfo" depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST + depends on PAHOLE_HAS_ZEROSIZE_PERCPU_SUPPORT help Generate deduplicated BTF type information from DWARF debug info. Turning this on expects presence of pahole tool, which will convert
This patch replaces mm-page_alloc-convert-per-cpu-list-protection-to-local_lock-fix.patch in Andrew's tree. Michal Suchanek reported the following problem with linux-next [ 0.000000] Linux version 5.13.0-rc2-next-20210519-1.g3455ff8-vanilla (geeko@buildhost) (gcc (SUSE Linux) 10.3.0, GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.36.1.20210326-3) #1 SMP Wed May 19 10:05:10 UTC 2021 (3455ff8) [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-5.13.0-rc2-next-20210519-1.g3455ff8-vanilla root=UUID=ec42c33e-a2c2-4c61-afcc-93e9527 8f687 plymouth.enable=0 resume=/dev/disk/by-uuid/f1fe4560-a801-4faf-a638-834c407027c7 mitigations=auto earlyprintk initcall_debug nomodeset earlycon ignore_loglevel console=ttyS0,115200 ... [ 26.093364] calling tracing_set_default_clock+0x0/0x62 @ 1 [ 26.098937] initcall tracing_set_default_clock+0x0/0x62 returned 0 after 0 usecs [ 26.106330] calling acpi_gpio_handle_deferred_request_irqs+0x0/0x7c @ 1 [ 26.113033] initcall acpi_gpio_handle_deferred_request_irqs+0x0/0x7c returned 0 after 3 usecs [ 26.121559] calling clk_disable_unused+0x0/0x102 @ 1 [ 26.126620] initcall clk_disable_unused+0x0/0x102 returned 0 after 0 usecs [ 26.133491] calling regulator_init_complete+0x0/0x25 @ 1 [ 26.138890] initcall regulator_init_complete+0x0/0x25 returned 0 after 0 usecs [ 26.147816] Freeing unused decrypted memory: 2036K [ 26.153682] Freeing unused kernel image (initmem) memory: 2308K [ 26.165776] Write protecting the kernel read-only data: 26624k [ 26.173067] Freeing unused kernel image (text/rodata gap) memory: 2036K [ 26.180416] Freeing unused kernel image (rodata/data gap) memory: 1184K [ 26.187031] Run /init as init process [ 26.190693] with arguments: [ 26.193661] /init [ 26.195933] with environment: [ 26.199079] HOME=/ [ 26.201444] TERM=linux [ 26.204152] BOOT_IMAGE=/boot/vmlinuz-5.13.0-rc2-next-20210519-1.g3455ff8-vanilla [ 26.254154] BPF: type_id=35503 offset=178440 size=4 [ 26.259125] BPF: [ 26.261054] BPF:Invalid offset [ 26.264119] BPF: [ 26.264119] [ 26.267437] failed to validate module [efivarfs] BTF: -22 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 <msuchanek@suse.de> Reported-by: Hritik Vijay <hritikxx8@gmail.com> Debugged-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- lib/Kconfig.debug | 4 ++++ 1 file changed, 4 insertions(+)