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;
From:   Zhangjin Wu <falcon@tinylab.org>
To:     w@1wt.eu
Cc:     arnd@arndb.de, david.laight@aculab.com, falcon@tinylab.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        tanyuan@tinylab.org, thomas@t-8ch.de
Subject: Re: [RFC] tools/nolibc: replace duplicated -ENOSYS return with single -ENOSYS return
Date:   Wed, 30 Aug 2023 08:19:07 +0800
Message-Id: <20230830001907.67499-1-falcon@tinylab.org>
In-Reply-To: <ZO2QWPYpo1fdXjX+@1wt.eu>
References: <ZO2QWPYpo1fdXjX+@1wt.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Feedback-ID: bizesmtp:tinylab.org:qybglogicsvrgz:qybglogicsvrgz5a-1
Precedence: bulk

Hi, Thomas, Willy and David

> Hi all,
> 
> On Sun, Aug 27, 2023 at 11:17:19AM +0200, Thomas Wei?schuh wrote:
> > To be honest I don't see a problem with the current aproach.
> > It is very obvious what is going on, the same pattern is used by other
> > projects and the "overhead" is very small.
> > 
> > 
> > It seems the macros will only work for simple cases which only test the
> > availability of a single syscall number.
> >

Good news, as I just replied [1] and as the test [2] shows, the
__stringify() based __is_nr_defined() macro (proposed in this RFC
thread) can test syscall definition for us, and the __stringify() and
__nrtoi() macro (Derive from David's NR_toi() work [3],[4]) based
__get_nr() macro can get the syscall number constant, both of them are
compiling stage cost and the compiling is not slow for all of the
current supported architectures, no size cost no runtime cost and even
help us to get smaller binary ;-)

[1]: https://lore.kernel.org/lkml/20230829233912.63097-1-falcon@tinylab.org/
[2]: https://godbolt.org/z/a7hxWj83E
[3]: https://lore.kernel.org/lkml/6819b8e273dc44e18f14be148549b828@AcuMS.aculab.com/
[4]: https://godbolt.org/z/rear4c1hj

> > Of these we currently only have:
> > gettimeofday(), lseek(), statx(), wait4()
> > 
> > So in it's current form we save 4 * 4 = 16 lines of code.
> > The proposed solution introduces 14 + 2 (empty) = 16 lines of new code,
> > and a bunch of mental overhead.
> >

Yes, as also suggested by Willy, the old proposed method redefined
NOLIBC__NR_* macros for every __NR_* and it must be avoided, and now,
the __is_nr_defined() and __get_nr() macros will simply avoid defining
new NOLIBC__NR_* for exisitng __NR_*, they can be used to test and get
the existing __NR_* directly.

In my local repo, we have saved 500+ lines ;-)

    $ git show nolibc/next:tools/include/nolibc/sys.h | wc -l
    1190
    $ cat tools/include/nolibc/sys.h | wc -l
    690

Including all of the -ENOSYS and #ifdef's:

    $ git grep -r ENOSYS nolibc/next:tools/include/nolibc/sys.h | wc -l
    17
    $ git grep -Er "#ifdef|#el|#endif" nolibc/next:tools/include/nolibc/sys.h | wc -l
    77

> > In case multiple underlying syscalls can be used these take different
> > arguments which a simple macro won't be able to encode sanely.
> 
> I totally agree, I would prefer all this to be manageable by humans with
> no preprocessor brain implant as much as possible as well.
>

Yeah, for the sys_* definitions, it is ok for us to use explicit arguments
intead of the '...'/__VA_ARGS__ to avoid losing some arguments sometimes, let's
do it in the RFC patchset but it should come after the tinyconfig patchset.

BTW, Willy, when will you prepare the branch for v6.7 developmlent? ;-)

Thanks,
Zhangjin

> Thanks,
> Willy