linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Mark Rutland <mark.rutland@arm.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
	linux-kernel@vger.kernel.org, valentin.schneider@arm.com
Subject: Re: rcutorture initrd/nolibc build on ARMv8?
Date: Wed, 20 Jan 2021 15:25:00 +0100	[thread overview]
Message-ID: <20210120142500.GB15935@1wt.eu> (raw)
In-Reply-To: <20210120134511.GA77728@C02TD0UTHF1T.local>

On Wed, Jan 20, 2021 at 01:45:11PM +0000, Mark Rutland wrote:
> > Ah that's very interesting indeed because actually these ones should
> > only be used when __NR_dup3 or __NR_clone are not defined. Thus I wanted
> > to check the definitions that were reported in your error output but
> > actually what was needed was to figure whether the correct ones were
> > present, and they are, here on my machine (and yes I agree that in this
> > case the dup2/fork above are bofus):
> 
> The issue is that even if a function is unused, the compiler still has
> to parse and compile the code, so where __NR_dup2 is not defined, we'll
> get a build error for:
> 
> static __attribute__((unused))
> int sys_dup2(int old, int new)
> {
>        return my_syscall2(__NR_dup2, old, new);
> }

For sure but this is supposed to be used only when __NR_dup3 is not
defined. Ah now I understand where my mistake is: after it built
successfully for me I inspected the most recent tree which has these
in place. Sorry for being stupid!

In my local tree it's defined like this:

 static __attribute__((unused))
 int sys_dup2(int old, int new)
 {
#ifdef __NR_dup3
       return my_syscall3(__NR_dup3, old, new, 0);
#else
       return my_syscall2(__NR_dup2, old, new);
#endif
 }

I guess I fixed it in a hurry and forgot to upstream it. I hate doing
that :-(

I'm going to push an update then. On a quick glance I'm seeing that I
addressed dup2() using dup3(), fork() using clone(), getpgrp() using
getpgid(), and poll() using ppoll().

> ... we can deal with that by always returning -ENOSYS for unimplemented
> syscalls, e.g.
> 
> static __attribute__((unused))
> int sys_dup2(int old, int new)
> {
> #ifdef __NR_dup2
>        return my_syscall2(__NR_dup2, old, new);
> #else
>        return -ENOSYS;
> #endif
> }

I didn't want to do that because that would break userland which needs
dup2(), hence the mapping to dup3 instead:

  static __attribute__((unused))
  int sys_dup2(int old, int new)
  {
  #ifdef __NR_dup3
          return my_syscall3(__NR_dup3, old, new, 0);
  #else
          return my_syscall2(__NR_dup2, old, new);
  #endif
  }

> I can spin a patch fixing up all the relevant syscalls, if you'd like?

I shouldn't need since these are already fixed in my tree. At first glance
the equivalent of the following commits is missing from the kernel version:

   https://github.com/wtarreau/nolibc/commit/2379f25073f906d0bad22c48566fcffee8fc9cde
   https://github.com/wtarreau/nolibc/commit/fd5272ec2c66e6f5b195d41c9c8978f58eb74668
   https://github.com/wtarreau/nolibc/commit/47cc42a79c92305f4f8bc02fb28628a4fdd63eaa
   https://github.com/wtarreau/nolibc/commit/d2dc42fd614991c741dfdc8b984864fa3cf64c8e
   https://github.com/wtarreau/nolibc/commit/800f75c13ede49097325f82a4cca3515c44a7939

However I'm interested in knowing if the latest version fixes everything
for you or not :

  https://raw.githubusercontent.com/wtarreau/nolibc/master/nolibc.h

> [...]
> 
> >   ubuntu@ubuntu:~$ gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)'
> >   #define __NR_clone 220
> >   #define __NR_dup3 24
> > 
> > Do you have these ones with your more recent includes ? Or are these wrong
> > again ?
> 
> Those are correct (and all the syscall numbers in unistd.h should be
> correct so long as you don't erroneously set the __ARCH_WANT_* flags):
> 
> [mark@gravadlaks:~/src/linux]% gcc -fno-asynchronous-unwind-tables -fno-ident -nostdlib -include tools/include/nolibc/nolibc.h -lgcc -s -static -E -dM init-fail.c | egrep '__NR_(clone|dup3)'
> #define __NR_clone 220
> #define __NR_dup3 24

OK thanks! I will retry here without setting those. I'm pretty sure I
needed these ones to find the __NR_* values but it's possible that it
was before I had the alternate ones and that these are finally not
nedeed at all (which I would prefer as these are ugly).

> There's still some latent issue when using nolibc (compared to using
> glibc) where the init process never seems to exit, but that looks to be
> orthogonal to the syscall numbering issue -- I'm currently digging into
> that.

OK! Usually for me it does as in my preinit (which uses nolibc), if I
exit I instantly get a kernel panic. In addition if I launch it after
boot, it immediately exits and shows no issue. But maybe you're observing
an artefact related to something else (process session, opened FD or
anything else maybe).

I'll send an update ASAP, likely this evening.

Many thanks for digging into this, and sorry for this mess, as I was
absolutely certain it was up to date :-(

Willy

  reply	other threads:[~2021-01-20 14:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-19 15:31 rcutorture initrd/nolibc build on ARMv8? Paul E. McKenney
2021-01-19 16:19 ` Willy Tarreau
2021-01-19 17:02   ` Mark Rutland
2021-01-19 17:16     ` Willy Tarreau
2021-01-19 17:43       ` Willy Tarreau
2021-01-20 12:07         ` Mark Rutland
2021-01-20 12:43           ` Willy Tarreau
2021-01-20 13:45             ` Mark Rutland
2021-01-20 14:25               ` Willy Tarreau [this message]
2021-01-20 14:37                 ` Mark Rutland
2021-01-20 14:54                 ` Mark Rutland
2021-01-20 15:02                   ` Willy Tarreau
2021-01-21  3:50                     ` Willy Tarreau
2021-02-12 12:37 ` [tip: core/rcu] tools/nolibc: Remove incorrect definitions of __ARCH_WANT_* tip-bot2 for Willy Tarreau

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 \
    --in-reply-to=20210120142500.GB15935@1wt.eu \
    --to=w@1wt.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=paulmck@kernel.org \
    --cc=valentin.schneider@arm.com \
    /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;
as well as URLs for NNTP newsgroup(s).