From: Alistair Francis <alistair23@gmail.com>
To: GNU C Library <libc-alpha@sourceware.org>
Cc: Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH v2] sysdeps/wait: Use the waitid syscall if required
Date: Tue, 12 Nov 2019 09:35:52 -0800 [thread overview]
Message-ID: <CAKmqyKP6h3Z_VRcz5471TyS+KYPzOr7YuiegRqZkzP_so_vodg@mail.gmail.com> (raw)
In-Reply-To: <CAKmqyKM4yvgE8L07U17efTFok-s3Nx5ZHG3bsrEVvogbh=0vjA@mail.gmail.com>
On Wed, Nov 6, 2019 at 3:47 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Nov 1, 2019 at 12:08 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Fri, Oct 25, 2019 at 6:01 PM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > If the waitpid and wait4 syscalls aren't avaliable (such as y2038 safe
> > > 32-bit systems) let us use the waitid syscall isntead.
> > >
> > > Unfortunately waitid is substantially differnt to waitpid and wait4, so
> > > the conversion ends up being complex.
> > >
> > > For full support we need the 5.4+ kernel as that allows a pid of 0 with
> > > the P_PGID idtype.
> > >
> > > This change also removes the wait4 syscall from syscalls.list and
> > > replaces it with a __wait4() implementation.
> > > ---
> > > This was patch was runtime tested with RV32 and RV64
> > > It was build tested using the ./scripts/build-many-glibcs.py script.
> >
> > Ping!
>
> Ping^2
Ping^3
Alistair
>
> Alistair
>
> >
> > Alistair
> >
> > >
> > > include/sys/wait.h | 3 +-
> > > sysdeps/unix/sysv/linux/syscalls.list | 1 -
> > > sysdeps/unix/sysv/linux/wait.c | 41 +++++++++-
> > > sysdeps/unix/sysv/linux/wait4.c | 87 ++++++++++++++++++++++
> > > sysdeps/unix/sysv/linux/waitpid.c | 59 ++++++++++++++-
> > > sysdeps/unix/sysv/linux/waitpid_nocancel.c | 56 +++++++++++++-
> > > 6 files changed, 239 insertions(+), 8 deletions(-)
> > > create mode 100644 sysdeps/unix/sysv/linux/wait4.c
> > >
> > > diff --git a/include/sys/wait.h b/include/sys/wait.h
> > > index 5ac9cd6ca6b..fab0e17f7d5 100644
> > > --- a/include/sys/wait.h
> > > +++ b/include/sys/wait.h
> > > @@ -13,7 +13,6 @@ extern __pid_t __wait (int *__stat_loc);
> > > extern __pid_t __wait3 (int *__stat_loc,
> > > int __options, struct rusage * __usage);
> > > extern __pid_t __wait4 (__pid_t __pid, int *__stat_loc,
> > > - int __options, struct rusage *__usage)
> > > - attribute_hidden;
> > > + int __options, struct rusage *__usage);
> > > #endif
> > > #endif
> > > diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> > > index e374f97b5f8..31f1d258fe1 100644
> > > --- a/sysdeps/unix/sysv/linux/syscalls.list
> > > +++ b/sysdeps/unix/sysv/linux/syscalls.list
> > > @@ -69,7 +69,6 @@ swapoff - swapoff i:s __swapoff swapoff
> > > unshare EXTRA unshare i:i unshare
> > > uselib EXTRA uselib i:s __compat_uselib uselib@GLIBC_2.0:GLIBC_2.23
> > > utime - utime i:sP utime
> > > -wait4 - wait4 i:iWiP __wait4 wait4
> > >
> > > chown - chown i:sii __libc_chown __chown chown
> > >
> > > diff --git a/sysdeps/unix/sysv/linux/wait.c b/sysdeps/unix/sysv/linux/wait.c
> > > index c2385c752e2..28a27af8135 100644
> > > --- a/sysdeps/unix/sysv/linux/wait.c
> > > +++ b/sysdeps/unix/sysv/linux/wait.c
> > > @@ -26,9 +26,44 @@
> > > pid_t
> > > __libc_wait (int *stat_loc)
> > > {
> > > - pid_t result = SYSCALL_CANCEL (wait4, WAIT_ANY, stat_loc, 0,
> > > - (struct rusage *) NULL);
> > > - return result;
> > > +#ifdef __NR_wait4
> > > + return SYSCALL_CANCEL (wait4, WAIT_ANY, stat_loc, 0,
> > > + (struct rusage *) NULL);
> > > +#else
> > > + siginfo_t infop;
> > > + __pid_t ret;
> > > +
> > > + ret = SYSCALL_CANCEL (waitid, P_ALL, 0, &infop, WEXITED, NULL);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (stat_loc)
> > > + {
> > > + *stat_loc = 0;
> > > + switch (infop.si_code)
> > > + {
> > > + case CLD_EXITED:
> > > + *stat_loc = infop.si_status << 8;
> > > + break;
> > > + case CLD_DUMPED:
> > > + *stat_loc = 0x80;
> > > + /* Fallthrough */
> > > + case CLD_KILLED:
> > > + *stat_loc |= infop.si_status;
> > > + break;
> > > + case CLD_TRAPPED:
> > > + case CLD_STOPPED:
> > > + *stat_loc = infop.si_status << 8 | 0x7f;
> > > + break;
> > > + case CLD_CONTINUED:
> > > + *stat_loc = 0xffff;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return infop.si_pid;
> > > +#endif
> > > }
> > >
> > > weak_alias (__libc_wait, __wait)
> > > diff --git a/sysdeps/unix/sysv/linux/wait4.c b/sysdeps/unix/sysv/linux/wait4.c
> > > new file mode 100644
> > > index 00000000000..6d6fea34f9a
> > > --- /dev/null
> > > +++ b/sysdeps/unix/sysv/linux/wait4.c
> > > @@ -0,0 +1,87 @@
> > > +/* Linux wait4 syscall implementation.
> > > + Copyright (C) 1991-2019 Free Software Foundation, Inc.
> > > + This file is part of the GNU C Library.
> > > +
> > > + The GNU C Library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later version.
> > > +
> > > + The GNU C Library is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General Public
> > > + License along with the GNU C Library; if not, see
> > > + <https://www.gnu.org/licenses/>. */
> > > +
> > > +#include <sys/wait.h>
> > > +#include <errno.h>
> > > +#include <sys/resource.h>
> > > +#include <stddef.h>
> > > +
> > > +__pid_t
> > > +__wait4 (__pid_t pid, int *stat_loc, int options,
> > > + struct rusage *usage)
> > > +{
> > > +#ifdef __NR_wait4
> > > + return INLINE_SYSCALL_CALL (wait4, pid, stat_loc, options, usage);
> > > +#else
> > > + __pid_t ret;
> > > + idtype_t idtype = P_PID;
> > > + siginfo_t infop;
> > > +
> > > + if (pid < -1)
> > > + {
> > > + idtype = P_PGID;
> > > + pid *= -1;
> > > + }
> > > + else if (pid == -1)
> > > + {
> > > + idtype = P_ALL;
> > > + }
> > > + else if (pid == 0)
> > > + {
> > > + /* Linux Kernels 5.4+ support pid 0 with P_PGID to specify wait on
> > > + * the current PID's group. Earlier kernels will return -EINVAL.
> > > + */
> > > + idtype = P_PGID;
> > > + }
> > > +
> > > + options |= WEXITED;
> > > +
> > > + ret = INLINE_SYSCALL_CALL (waitid, idtype, pid, &infop, options, usage);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (stat_loc)
> > > + {
> > > + *stat_loc = 0;
> > > + switch (infop.si_code)
> > > + {
> > > + case CLD_EXITED:
> > > + *stat_loc = infop.si_status << 8;
> > > + break;
> > > + case CLD_DUMPED:
> > > + *stat_loc = 0x80;
> > > + /* Fallthrough */
> > > + case CLD_KILLED:
> > > + *stat_loc |= infop.si_status;
> > > + break;
> > > + case CLD_TRAPPED:
> > > + case CLD_STOPPED:
> > > + *stat_loc = infop.si_status << 8 | 0x7f;
> > > + break;
> > > + case CLD_CONTINUED:
> > > + *stat_loc = 0xffff;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return infop.si_pid;
> > > +#endif
> > > +}
> > > +
> > > +weak_alias (__wait4, wait4)
> > > diff --git a/sysdeps/unix/sysv/linux/waitpid.c b/sysdeps/unix/sysv/linux/waitpid.c
> > > index d35aac01bcc..a02275c3ff5 100644
> > > --- a/sysdeps/unix/sysv/linux/waitpid.c
> > > +++ b/sysdeps/unix/sysv/linux/waitpid.c
> > > @@ -20,14 +20,71 @@
> > > #include <sysdep-cancel.h>
> > > #include <stdlib.h>
> > > #include <sys/wait.h>
> > > +#include <unistd.h>
> > >
> > > __pid_t
> > > __waitpid (__pid_t pid, int *stat_loc, int options)
> > > {
> > > #ifdef __NR_waitpid
> > > return SYSCALL_CANCEL (waitpid, pid, stat_loc, options);
> > > -#else
> > > +#elif defined(__NR_wait4)
> > > return SYSCALL_CANCEL (wait4, pid, stat_loc, options, NULL);
> > > +#else
> > > + __pid_t ret;
> > > + idtype_t idtype = P_PID;
> > > + siginfo_t infop;
> > > +
> > > + if (pid < -1)
> > > + {
> > > + idtype = P_PGID;
> > > + pid *= -1;
> > > + }
> > > + else if (pid == -1)
> > > + {
> > > + idtype = P_ALL;
> > > + }
> > > + else if (pid == 0)
> > > + {
> > > + /* Linux Kernels 5.4+ support pid 0 with P_PGID to specify wait on
> > > + * the current PID's group. Earlier kernels will return -EINVAL.
> > > + */
> > > + idtype = P_PGID;
> > > + }
> > > +
> > > + options |= WEXITED;
> > > +
> > > + ret = SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, NULL);
> > > +
> > > + if (ret < 0)
> > > + {
> > > + return ret;
> > > + }
> > > +
> > > + if (stat_loc)
> > > + {
> > > + *stat_loc = 0;
> > > + switch (infop.si_code)
> > > + {
> > > + case CLD_EXITED:
> > > + *stat_loc = infop.si_status << 8;
> > > + break;
> > > + case CLD_DUMPED:
> > > + *stat_loc = 0x80;
> > > + /* Fallthrough */
> > > + case CLD_KILLED:
> > > + *stat_loc |= infop.si_status;
> > > + break;
> > > + case CLD_TRAPPED:
> > > + case CLD_STOPPED:
> > > + *stat_loc = infop.si_status << 8 | 0x7f;
> > > + break;
> > > + case CLD_CONTINUED:
> > > + *stat_loc = 0xffff;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return infop.si_pid;
> > > #endif
> > > }
> > > libc_hidden_def (__waitpid)
> > > diff --git a/sysdeps/unix/sysv/linux/waitpid_nocancel.c b/sysdeps/unix/sysv/linux/waitpid_nocancel.c
> > > index 3697c6b938c..59b07c5f73d 100644
> > > --- a/sysdeps/unix/sysv/linux/waitpid_nocancel.c
> > > +++ b/sysdeps/unix/sysv/linux/waitpid_nocancel.c
> > > @@ -27,8 +27,62 @@ __waitpid_nocancel (__pid_t pid, int *stat_loc, int options)
> > > {
> > > #ifdef __NR_waitpid
> > > return INLINE_SYSCALL_CALL (waitpid, pid, stat_loc, options);
> > > -#else
> > > +#elif defined (__NR_wait4)
> > > return INLINE_SYSCALL_CALL (wait4, pid, stat_loc, options, NULL);
> > > +#else
> > > + __pid_t ret;
> > > + idtype_t idtype = P_PID;
> > > + siginfo_t infop;
> > > +
> > > + if (pid < -1)
> > > + {
> > > + idtype = P_PGID;
> > > + pid *= -1;
> > > + }
> > > + else if (pid == -1)
> > > + {
> > > + idtype = P_ALL;
> > > + }
> > > + else if (pid == 0)
> > > + {
> > > + /* Linux Kernels 5.4+ support pid 0 with P_PGID to specify wait on
> > > + * the current PID's group. Earlier kernels will return -EINVAL.
> > > + */
> > > + idtype = P_PGID;
> > > + }
> > > +
> > > + options |= WEXITED;
> > > +
> > > + ret = INLINE_SYSCALL_CALL (waitid, idtype, pid, &infop, options, NULL);
> > > +
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + if (stat_loc)
> > > + {
> > > + *stat_loc = 0;
> > > + switch (infop.si_code)
> > > + {
> > > + case CLD_EXITED:
> > > + *stat_loc = infop.si_status << 8;
> > > + break;
> > > + case CLD_DUMPED:
> > > + *stat_loc = 0x80;
> > > + /* Fallthrough */
> > > + case CLD_KILLED:
> > > + *stat_loc |= infop.si_status;
> > > + break;
> > > + case CLD_TRAPPED:
> > > + case CLD_STOPPED:
> > > + *stat_loc = infop.si_status << 8 | 0x7f;
> > > + break;
> > > + case CLD_CONTINUED:
> > > + *stat_loc = 0xffff;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + return infop.si_pid;
> > > #endif
> > > }
> > > libc_hidden_def (__waitpid_nocancel)
> > > --
> > > 2.23.0
> > >
next prev parent reply other threads:[~2019-11-12 17:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-25 15:56 [PATCH v2] sysdeps/wait: Use the waitid syscall if required Alistair Francis
2019-11-01 7:08 ` Alistair Francis
2019-11-06 23:47 ` Alistair Francis
2019-11-12 17:35 ` Alistair Francis [this message]
2019-11-13 21:00 ` Adhemerval Zanella
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
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKP6h3Z_VRcz5471TyS+KYPzOr7YuiegRqZkzP_so_vodg@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=libc-alpha@sourceware.org \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).