Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 16 Jul 2023 06:50:15 +0200
From:   Willy Tarreau <w@1wt.eu>
To:     Zhangjin Wu <falcon@tinylab.org>
Cc:     arnd@arndb.de, linux-kernel@vger.kernel.org,
        linux-kselftest@vger.kernel.org, thomas@t-8ch.de
Subject: Re: [PATCH v4 00/18] tools/nolibc: shrink arch support
Message-ID: <20230716045015.GA28761@1wt.eu>
References: <20230715222658.GA27708@1wt.eu>
 <20230716011744.499597-1-falcon@tinylab.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230716011744.499597-1-falcon@tinylab.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Sun, Jul 16, 2023 at 09:17:44AM +0800, Zhangjin Wu wrote:
> > I finally found that it's due to the lack of -Isysroot/x86/include, so
> > it used to get linux includes from those provided by glibc and these ones
> > were missing statx since packaged for an older kernel.
> >
> 
> So, your local glibc may be older than 2.28 (The one we mentioned in the
> commit message who supports statx)? mine 2.31 glibc is ok:

Oh definitely! It's a 2.23, and on another machine I'm having a 2.27
on an ubuntu 18 but it was built against a more recent kernel so its
linux/stat.h has the required entries, and on another one I'm having
a 2.17 which was built against a 3.10 kernel.

> For older Linux systems without a newer libc may really require the
> installation of the linux sysroot (linux/uapi).

Yes. My point was that it wasn't very hard to already spot breakage
on existing code built on existing setups.

> > I knew that sooner or later I'd have to reinstall this machine but I
> > can't get out of my head that to date I have yet not been convinced by
> > the absolute necessity of this modification which is progressively adding
> > more burden :-/  Time will tell...
> >
> 
> This may also let us think about the removing of <linux/xxx.h> from our
> nolibc headers? just like musl does ;-)
> 
>     $ grep "include <linux" -ur ../../../include/nolibc/
>     ../../../include/nolibc/stdlib.h:#include <linux/auxvec.h>
>     ../../../include/nolibc/sys.h:#include <linux/fs.h>
>     ../../../include/nolibc/sys.h:#include <linux/loop.h>
>     ../../../include/nolibc/sys.h:#include <linux/time.h>
>     ../../../include/nolibc/sys.h:#include <linux/auxvec.h>
>     ../../../include/nolibc/sys.h:#include <linux/fcntl.h> /* for O_* and AT_* */
>     ../../../include/nolibc/sys.h:#include <linux/stat.h>  /* for statx() */
>     ../../../include/nolibc/sys.h:#include <linux/prctl.h>
>     ../../../include/nolibc/types.h:#include <linux/mman.h>
>     ../../../include/nolibc/types.h:#include <linux/reboot.h> /* for LINUX_REBOOT_* */
>     ../../../include/nolibc/types.h:#include <linux/stat.h>
>     ../../../include/nolibc/types.h:#include <linux/time.h>
> 
> If simply put all of them to types.h, it may be too much, a new "sys/"
> directory with almost the same Linux type files may be required, but as
> an in-kernel libc, this duplication may be a "big" issue too, so, adding
> minimal required macros and structs in types.h may be another choice.
(...)
> The required new macros and structs may be around 100-300 lines? but it may
> help to avoid the installation of sysroot completely and also avoid the cross
> including the linux-libc-dev package used by glibc?

No, really, that's what we used to do previously. If you remember we
recently removed lots of structs and defines from various files because
they used to regularly conflict with linux/foo.h (that we can't prevent
users from including), while not always being 100% up-to-date. It's
particularly annoying when there are typedefs for example because it's
difficult to detect them, and if you redefine them you end up with build
errors. We should only keep that for absolute necessity. In fact, maybe
we could have these few ones precisely for statx, right after including
linux/stat.h (which is supposed to provide them):

  #ifndef STATX_BASIC_STATS
    /* pre-4.10 linux uapi headers present, missing statx that we need */
    #define STATX_BASIC_STATS xxx
    struct statx {
        ...
    };
  #endif

I may give this a try to see if it's sufficient to fix the build on
these machines. But it's not critical anyway. I might try once I'm
bored of seeing build failures.

Cheers,
Willy