Message-ID: <CAD=FV=V355McabzO0sEwy_bbYZjPTGF=8iLU9wqQWDiFg9h-DQ@mail.gmail.com>
Date: Sun, 23 Mar 2025 19:02:44 -0700
From: Doug Anderson <dianders@...omium.org>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Frank Binns <frank.binns@...tec.com>, Matt Coster <matt.coster@...tec.com>, 
	linux-kernel@...r.kernel.org, Arnaldo Carvalho de Melo <acme@...hat.com>, 
	Borislav Petkov <bp@...e.de>, Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>, 
	bpf@...r.kernel.org, linux-kbuild@...r.kernel.org, 
	Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH] tools: fix annoying "mkdir -p ..." logs when building
 tools in parallel

Hi,

On Sat, Mar 22, 2025 at 7:50 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Wed, Mar 19, 2025 at 1:41 AM Doug Anderson <dianders@...omium.org> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 10, 2025 at 4:30 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
> > >
> > > When CONFIG_OBJTOOL=y or CONFIG_DEBUG_INFO_BTF=y, parallel builds
> > > show awkward "mkdir -p ..." logs.
> > >
> > >   $ make -j16
> > >     [ snip ]
> > >   mkdir -p /home/masahiro/ref/linux/tools/objtool && make O=/home/masahiro/ref/linux subdir=tools/objtool --no-print-directory -C objtool
> > >   mkdir -p /home/masahiro/ref/linux/tools/bpf/resolve_btfids && make O=/home/masahiro/ref/linux subdir=tools/bpf/resolve_btfids --no-print-directory -C bpf/resolve_btfids
> > >
> > > Defining MAKEFLAGS=<value> on the command line wipes out command line
> > > switches from the resultant MAKEFLAGS definition, even though the command
> > > line switches are active. [1]
> > >
> > > The first word of $(MAKEFLAGS) is a possibly empty group of characters
> > > representing single-letter options that take no argument. However, this
> > > breaks if MAKEFLAGS=<value> is given on the command line.
> > >
> > > The tools/ and tools/% targets set MAKEFLAGS=<value> on the command
> > > line, which breaks the following code in tools/scripts/Makefile.include:
> > >
> > >     short-opts := $(firstword -$(MAKEFLAGS))
> > >
> > > If MAKEFLAGS really needs modification, it should be done through the
> > > environment variable, as follows:
> > >
> > >     MAKEFLAGS=<value> $(MAKE) ...
> > >
> > > That said, I question whether modifying MAKEFLAGS is necessary here.
> > > The only flag we might want to exclude is --no-print-directory, as the
> > > tools build system changes the working directory. However, people might
> > > find the "Entering/Leaving directory" logs annoying.
> > >
> > > I simply removed the offending MAKEFLAGS=.
> > >
> > > [1]: https://savannah.gnu.org/bugs/?62469
> > >
> > > Fixes: a50e43332756 ("perf tools: Honor parallel jobs")
> > > Signed-off-by: Masahiro Yamada <masahiroy@...nel.org>
> > > ---
> > >
> > >  Makefile | 9 ++-------
> > >  1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > I happened to sync up to mainline today and noticed that my build was
> > broken. I bisected it to this change and reverting this change fixes
> > my build on mainline.
> >
> > In my case I'm building in a ChromeOS environment and using clang as
> > my compiler. I'm also cross-compiling an arm64 kernel on x86 host.
> > ...but the pure mainline kernel should work there. Presumably the
> > environment is a bit different compared to the typical one, though?
> >
> > The error comes up when doing a clean build and the first error messages are:
> >
> > In file included from libbpf.c:36:
> > .../tools/include/uapi/linux/bpf_perf_event.h:14:21: error: field has
> > incomplete type
> >       'bpf_user_pt_regs_t' (aka 'struct user_pt_regs')
> >    14 |         bpf_user_pt_regs_t regs;
> >       |                            ^
> > .../tools/include/../../arch/arm64/include/uapi/asm/bpf_perf_event.h:7:16:
> > note: forward
> >       declaration of 'struct user_pt_regs'
> >     7 | typedef struct user_pt_regs bpf_user_pt_regs_t;
> >       |                ^
> >
> > btf_dump.c:1860:10: error: cast to smaller integer type 'uintptr_t'
> > (aka 'unsigned int') from 'const void *'
> >       [-Werror,-Wvoid-pointer-to-int-cast]
> >  1860 |         return ((uintptr_t)data) % alignment == 0;
> >       |                 ^~~~~~~~~~~~~~~
> > btf_dump.c:2045:4: error: format specifies type 'ssize_t' (aka 'long')
> > but the argument has type 'ssize_t' (aka 'int')
> >       [-Werror,-Wformat]
> >  2044 |                 pr_warn("unexpected elem size %zd for array
> > type [%u]\n",
> >       |                                               ~~~
> >       |                                               %d
> >  2045 |                         (ssize_t)elem_size, id);
> >       |                         ^~~~~~~~~~~~~~~~~~
> > ./libbpf_internal.h:171:52: note: expanded from macro 'pr_warn'
> >   171 | #define pr_warn(fmt, ...)       __pr(LIBBPF_WARN, fmt, ##__VA_ARGS__)
> >       |                                                   ~~~    ^~~~~~~~~~~
> >
> >
> > I don't have time to dig right now, but I figured I'd at least post in
> > case the problem is obvious to someone else.
>
> I do not understand how to reproduce this.
>
> Your build machine is Chrome OS, or other distributions?
> Did you build the upstream kernel or downstream one?
> What is the build command?  Just "make" ?

Unfortunately, it's not a simple set of steps to reproduce and would
take a lot of time / disk space. :( Essentially, the steps are:

1. Setup a ChromeOS development environment. This is a pretty massive
thing. You'd have to follow the Developer Guide [1].

2. Enter the "chroot" in the development environment and build for a
board like "trogdor". This is an arm64 Chromebook.

3. The "trogdor" board normally builds a v6.6-based kernel, so it
looks for the kernel sources in `src/third_party/kernel/v6.6`. ...but
you can go into that folder and simply checkout a pure upstream kernel
as talked about in [2].

3. Inside the ChromeOS "chroot", you can do `cros-workon-trogdor start
chromeos-kernel-6_6` and `emerge-trogdor chromeos-kernel-6_6`. Since
you've checked out the upstream kernel to the v6.6 directory this
won't actually be building 6.6 but will be building the upstream
kernel.

...and that's where I see the error, which is "fixed" by reverting this patch.

FWIW: the actual build instructions for building the kernel are mostly
in cros-kernel.eclass [3].


Given how time consuming that would be to reproduce, I'm not sure it's
worth your time. Let me do some simple debugging... So I added these
extra printouts:

 tools/: FORCE
+       echo DOUGa new: "$(MAKEFLAGS)"
+       echo DOUGa old: "$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))"

 tools/%: FORCE
+       echo DOUGb new "$(MAKEFLAGS)"
+       echo DOUBb old: "$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))"

...and when I build, I see:

DOUGb new krR -j128 --jobserver-auth=3,4 --no-print-directory --
CLANG_CROSS_FLAGS=--target=aarch64-cros-linux-gnu
HOSTLD=x86_64-pc-linux-gnu-ld.lld
HOSTPKG_CONFIG=x86_64-pc-linux-gnu-pkg-config
HOSTCXX=x86_64-pc-linux-gnu-clang++ HOSTCC=x86_64-pc-linux-gnu-clang
CXX=aarch64-cros-linux-gnu-clang++\
CC_COMPAT=armv7a-cros-linux-gnueabihf-clang
CC=aarch64-cros-linux-gnu-clang\  AR=llvm-ar NM=llvm-nm
STRIP=llvm-strip REAL_STRIP=llvm-strip OBJCOPY=llvm-objcopy
LD_COMPAT=ld.lld LD=ld.lld
O=/build/trogdor/var/cache/portage/sys-kernel/chromeos-kernel-6_6 V=0

DOUBb old:  --jobserver-auth=3,4

...so pretty different! :-) I guess it used to be filtering out all
the cross-compiler info and now it isn't?


[1] https://www.chromium.org/chromium-os/developer-library/guides/development/developer-guide/
[2] https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/HEAD/eclass/cros-kernel/README.md
[3] https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/HEAD/eclass/cros-kernel.eclass

Powered by blists - more mailing lists