[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <O2kEogG8Ln8uUQ-ammarfaizi2@gnuweeb.org>
Date: Mon, 18 Oct 2021 13:53:29 +0700
From: Ammar Faizi <ammar.faizi@...dents.amikom.ac.id>
To: Willy Tarreau <w@....eu>
Cc: Ammar Faizi <ammar.faizi@...dents.amikom.ac.id>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Andy Lutomirski <luto@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>,
David Laight <David.Laight@...LAB.COM>,
Peter Cordes <peter@...des.ca>,
Bedirhan KURT <windowz414@...weeb.org>,
Louvian Lyndal <louvianlyndal@...il.com>
Subject: Re: [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug
On Mon, Oct 18, 2021 at 11:59 AM Willy Tarreau <w@....eu> wrote:
> > Extra fixes:
> > - Use NR_exit_group instead of NR_exit.
>
> Please, this is independent on the fix above so it must be subject
> of a different patch with its own justification. Also it should cover
> all supported architectures, otherwise programs will start to behave
> differently on different targets.
>
> > - Use `mov %eax,%edi` instead of `movzbq %al,%rdi`. This makes the
> > exit code more observable from strace. While the exit code is
> > only 8-bit, the kernel has taken care of that, so no need to
> > worry about it.
>
> I'm fine with this one as well, but similarly, it should be in its own
> patch and applied to all architectures.
>
[...]
>
> I find the whole thing much less readable here, as asm code tends to
> be read as visual groups of patterns. I'm suggesting that you place a
> multi-line comment before the asm statement indicating the general
> rules (e.g. lowest stack frame must be zero, rsp+8 must be multiple of
> 16 etc), then only comment each instruction on the same line as it was
> before.
Got it, agree with that. I will address your review and resend this as a
patchset v2 soon.
--
Ammar Faizi
Powered by blists - more mailing lists