public inbox for [email protected]
 help / color / mirror / Atom feed
From: Ammar Faizi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: io-uring Mailing List <[email protected]>,
	GNU/Weeb Mailing List <[email protected]>,
	Pavel Begunkov <[email protected]>,
	Alviro Iskandar Setiawan <[email protected]>
Subject: Re: [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build
Date: Mon, 18 Apr 2022 21:14:20 +0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 4/18/22 9:01 AM, Jens Axboe wrote:
> On 4/14/22 4:41 PM, Ammar Faizi wrote:
>> Hi,
>>
>> This series adds nolibc support for x86 32-bit. There are 3 patches in
>> this series:
>>
>> 1) Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit.
>> 2) Provide `get_page_size()` function for x86 32-bit.
>> 3) Add x86 32-bit native syscall support.
>>
>> The most noticeable changes is the patch 3. Unlike x86-64, only
>> use native syscall from the __do_syscall macros when CONFIG_NOLIBC is
>> enabled for 32-bit build. The reason is because the libc syscall
>> wrapper can do better in 32-bit. The libc syscall wrapper can dispatch
>> the best syscall instruction that the environment is supported, there
>> are at least two variants syscall instruction for x86 32-bit, they are:
>> `int $0x80` and `sysenter`. The `int $0x80` instruction is always
>> available, but `sysenter` is not, it relies on VDSO. liburing always
>> uses `int $0x80` for syscall if it's compiled with CONFIG_NOLIBC,
>> otherwise it uses whatever the libc provides.
>>
>> Extra notes for __do_syscall6() macro:
>> On i386, the 6th argument of syscall goes in %ebp. However, both Clang
>> and GCC cannot use %ebp in the clobber list and in the "r" constraint
>> without using -fomit-frame-pointer. To make it always available for
>> any kind of compilation, the below workaround is implemented:
>>
>>    1) Push the 6-th argument.
>>    2) Push %ebp.
>>    3) Load the 6-th argument from 4(%esp) to %ebp.
>>    4) Do the syscall (int $0x80).
>>    5) Pop %ebp (restore the old value of %ebp).
>>    6) Add %esp by 4 (undo the stack pointer).
>>
>> WARNING:
>>    Don't use register variables for __do_syscall6(), there is a known
>>    GCC bug that results in an endless loop.
>>
>> BugLink: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105032
>>
>>
>> ===== How is this tested? =====
>>
>> This has been tested on x86-64 Linux (compiled with 32-bit bin support)
>> with the following commands:
>>
>>    sudo apt-get install gcc-i686-linux-gnu g++-i686-linux-gnu -y;
>>    ./configure --cc=i686-linux-gnu-gcc --cxx=i686-linux-gnu-g++ --nolibc;
>>    sudo make -j8 runtests;
> 
> Looks reasonable to me, even with the warts. I keep threatening to do a
> 2.2 release, and I do want to do that soon, so question is if we defer
> this patchset until after that has happened?
> 
> I'm looking for a gauge of confidence on the series.

I personally love not to defer this patchset. I understand that if we
were adding something like this to the Linux kernel, it's pretty sure
that it is not acceptable time. But liburing.

Several things that you may want to consider:

1) Previously, `--nolibc` build on x86 32-bit will throw a compile
    error, "Arch doesn't support building liburing without libc".
    After this patchset, it compiles just fine.

2) This series doesn't have any effect for x86 32-bit that uses libc,
    and that is what we do by default.

3) I believe x86 32-bit users are not that many. So having this one
    earlier gives sometime to get it mature without much chaos (if
    we ever found a bug).

    Not to say it's buggy. But younger code tend to be buggier. If we
    ever hit that bug due to this patchset, some of them may fallback
    to the libc build while waiting for the next stable liburing.

But anyway, I don't think it's that urgent seeing that we don't have
visible users that are actively using nolibc x86 32-bit. So if you
prefer to defer this, please defer it. What do you think?

-- 
Ammar Faizi

  reply	other threads:[~2022-04-18 15:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-14 22:41 [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 1/3] arch/syscall-defs: Use `__NR_mmap2` instead of `__NR_mmap` for x86 32-bit Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 2/3] arch/x86/lib: Provide `get_page_size()` function " Ammar Faizi
2022-04-14 22:41 ` [PATCH liburing 3/3] arch/x86/syscall: Add x86 32-bit native syscall support Ammar Faizi
2022-04-18  2:01 ` [PATCH liburing 0/3] Add x86 32-bit support for the nolibc build Jens Axboe
2022-04-18 14:14   ` Ammar Faizi [this message]
2022-04-18 15:23     ` Jens Axboe
2022-04-18 15:24 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox