Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210527120251.GC30378@techsingularity.net> <CAEf4BzartMG36AGs-7LkQdpgrB6TYyTJ8PQhjkQWiTN=7sO1Bw@mail.gmail.com>
 <20210527145441.GE30378@techsingularity.net> <CAEf4BzbW2i4Y-i3TXW7x42PqEpw5_nNeReSXS77m4GC3uqD3wg@mail.gmail.com>
 <20210527172700.GH30378@techsingularity.net>
In-Reply-To: <20210527172700.GH30378@techsingularity.net>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Thu, 27 May 2021 15:25:10 -0700
Message-ID: <CAEf4BzaP3ieYPX9vWETvp5bL8XRZ1XHr5C0dhLWed7DFanpa0g@mail.gmail.com>
Subject: Re: [PATCH v2] mm/page_alloc: Work around a pahole limitation with
 zero-sized struct pagesets
To:     Mel Gorman <mgorman@techsingularity.net>,
        Masahiro Yamada <masahiroy@kernel.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Christoph Hellwig <hch@infradead.org>,
        Arnaldo Carvalho de Melo <acme@redhat.com>,
        Michal Suchanek <msuchanek@suse.de>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Jiri Olsa <jolsa@kernel.org>,
        Hritik Vijay <hritikxx8@gmail.com>,
        Linux-BPF <bpf@vger.kernel.org>,
        Linux-Net <netdev@vger.kernel.org>, Linux-MM <linux-mm@kvack.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, May 27, 2021 at 10:27 AM Mel Gorman <mgorman@techsingularity.net> wrote:
>
> On Thu, May 27, 2021 at 09:36:35AM -0700, Andrii Nakryiko wrote:
> > On Thu, May 27, 2021 at 7:54 AM Mel Gorman <mgorman@techsingularity.net> wrote:
> > >
> > > On Thu, May 27, 2021 at 07:37:05AM -0700, Andrii Nakryiko wrote:
> > > > > This patch checks for older versions of pahole and only allows
> > > > > DEBUG_INFO_BTF_MODULES if pahole supports zero-sized per-cpu structures.
> > > > > DEBUG_INFO_BTF is still allowed as a KVM boot test passed with pahole
> > > >
> > > > Unfortunately this won't work. The problem is that vmlinux BTF is
> > > > corrupted, which results in module BTFs to be rejected as well, as
> > > > they depend on it.
> > > >
> > > > But vmlinux BTF corruption makes BPF subsystem completely unusable. So
> > > > even though kernel boots, nothing BPF-related works. So we'd need to
> > > > add dependency for DEBUG_INFO_BTF on pahole 1.22+.
> > > >
> > >
> > > While bpf usage would be broken, the kernel will boot and the effect
> > > should be transparent to any kernel build based on "make oldconfig".
> >
> > I think if DEBUG_INFO_BTF=y has no chance of generating valid vmlinux
> > BTF it has to be forced out. So if we are doing this at all, we should
> > do it for CONFIG_DEBUG_INFO_BTF, not CONFIG_DEBUG_INFO_BTF_MODULES.
> > CONFIG_DEBUG_INFO_BTF_MODULES will follow automatically.
> >
>
> Ok, I sent a version that prevents DEBUG_INFO_BTF being set unless
> pahole is at least 1.22.
>
> > > CONFIG_DEBUG_INFO_BTF defaults N so if that is forced out, it will be
> > > easily missed by a distribution kernel maintainer.
> >
> > We actually had previous discussions on forcing build failure in cases
> > when CONFIG_DEBUG_INFO_BTF=y can't be satisfied, but no one followed
> > up.
>
> It is weird how it is handled. DEBUG_INFO_BTF can be set and then fail to
> build vmlinux because pahole is too old. With DEBUG_INFO_BTF now requiring
> at least 1.22, the other version checks for 1.16 and 1.19 are redundant
> and could be cleaned up.
>
> > I'll look into this and will try to change the behavior. It's
> > caused too much confusion previously and now with changes like this we
> > are going to waste even more people's time.
> >
>
> Thanks.

So I've tried to change that, but I'm not sure that's possible with
the current Kconfig system. I tried to use $(error-if), but it happens
too early, at the text pre-processing stage, before the value of
CONFIG_DEBUG_INFO_BTF is known, so it's impossible to express
something like this:

$(error_if,CONFIG_DEBUG_INFO_BTF=y && PAHOLE_VERSION < 116,Pahole is tool old)

Masahiro, is it possible to somehow express the condition that if
CONFIG_DEBUG_INFO_BTF=y is selected, but some external dependency
(pahole version in this case) is too old, then fail the build
immediately? Currently we fail at the very end of vmlinux linking
step, which is very late.

Alternatively, it was proposed to just add an extra dependency (like,
"depends PAHOLE_IS_116_OR_NEWER"), but that will silently unselect
CONFIG_DEBUG_INFO_BTF if the condition is not satisfied, so it's even
more confusing to users.

Any suggestions on how to proceed with something like that? Thanks!

>
> --
> Mel Gorman
> SUSE Labs