This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC v3 03/23] sysdeps/wait: Use waitid if avaliable
- From: Alistair Francis <alistair23 at gmail dot com>
- To: Christian Brauner <christian at brauner dot io>
- Cc: "Eric W. Biederman" <ebiederm at xmission dot com>, Arnd Bergmann <arnd at arndb dot de>, Rich Felker <dalias at libc dot org>, Linus Torvalds <torvalds at linux-foundation dot org>, Alistair Francis <alistair dot francis at wdc dot com>, GNU C Library <libc-alpha at sourceware dot org>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, Florian Weimer <fweimer at redhat dot com>, Palmer Dabbelt <palmer at sifive dot com>, macro at wdc dot com, Zong Li <zongbox at gmail dot com>, Andrew Morton <akpm at linux-foundation dot org>, Al Viro <viro at zeniv dot linux dot org dot uk>, "H. Peter Anvin" <hpa at zytor dot com>
- Date: Tue, 13 Aug 2019 15:22:02 -0700
- Subject: Re: [RFC v3 03/23] sysdeps/wait: Use waitid if avaliable
- References: <CAK8P3a3wgavtarKxSYJGL0ME9KRZ8UsUAZw+Y5J8WpG1GQ+=mw@mail.gmail.com> <87muh79yt2.fsf@xmission.com> <CAHk-=wjtGaJFceL+YB6=mTxQCvyNvBTavqgGTm-d5FA9xLQ0Cw@mail.gmail.com> <87blxn83sk.fsf@xmission.com> <20190721232336.GA30851@brightrain.aerifal.cx> <87k1c962ml.fsf@xmission.com> <CAK8P3a0jOO8dDK+w0N_RvgUHiW7=i_ak9AyFvH61wqUusL3Drw@mail.gmail.com> <20190723082857.kf2go2vfvnu7q7zd@brauner.io> <CAK8P3a26KBRvJHyWkK0J8FGXQn4jHL2QG10oBGSLidG95xQxrw@mail.gmail.com> <CAKmqyKOXJxJq5-ktLc=oY7ooNj6-X9PotHNM=-xG95iPCAAmjQ@mail.gmail.com> <20190725044009.GJ1506@brightrain.aerifal.cx> <CAK8P3a1pjB8SMFQK0MUom0m5fCSAdVZFtJyeUkCLW-u-RHC3oA@mail.gmail.com> <8736iuujdu.fsf@xmission.com> <72291182-11E1-49C2-9608-875FD9473719@brauner.io>
On Thu, Jul 25, 2019 at 10:30 AM Christian Brauner <christian@brauner.io> wrote:
>
> On July 25, 2019 7:14:05 PM GMT+02:00, ebiederm@xmission.com wrote:
> >Arnd Bergmann <arnd@arndb.de> writes:
> >
> >> On Thu, Jul 25, 2019 at 6:40 AM Rich Felker <dalias@libc.org> wrote:
> >>> On Wed, Jul 24, 2019 at 05:04:53PM -0700, Alistair Francis wrote:
> >>> > On Tue, Jul 23, 2019 at 1:45 AM Arnd Bergmann <arnd@arndb.de>
> >wrote:
> >>> > >
> >>> > > Sounds good to me, the debate over what rusage to use should not
> >hold
> >>> > > up the review of the rest of that syscall.
> >>> >
> >>> > I'm unclear what the final decision is here. What is the solution
> >are
> >>> > we going to have wait4() or add P_PROCESS_PGID to waitid()?
> >>> >
> >>> > As well as that what is the solution to current implementations?
> >If we
> >>> > add wait4() then there isn't an issue (and I can drop this patch)
> >but
> >>> > if we add P_PROCESS_PGID then we will need a way to handle kernels
> >>> > with waitid() but no P_PROCESS_PGID. Although my new plan is to
> >only
> >>> > use the waitid syscall if we don't have waitpid or wait4 so it
> >seems
> >>> > like this will only affect RV32 for the time being.
> >>>
> >>> I would really like some indication which solution will be taken,
> >>> since it impacts choices that will need to be made in musl very
> >soon.
> >>> My favorite outcome would be bringing back wait4 for rv32 (and
> >>> no-time32 archs in general) *and* adding P_PROCESS_PGID. In the
> >short
> >>> term, just using wait4 would be the simplest and cleanest for us
> >(same
> >>> as all other archs, no extra case to deal with), but in the long
> >term
> >>> there may be value in having rusage that can represent more than 68
> >>> cpu-years spent by a process (seems plausible with large numbers of
> >>> cores).
> >>
> >> Based on the feedback from Linus and Eric, the most likely outcome
> >> at the moment seems to be an extension of waitid() to allow
> >> P_PGID with id=0 like BSD does, and not bring back wait4() or
> >> add P_PROCESS_PGID.
> >>
> >> So far, I don't think anyone has proposed an actual kernel patch.
> >> I was hoping that Eric would do it, but I could also send it if he's
> >> otherwise busy.
> >
> >So here is what I am looking at. It still needs to be tested
> >and the description needs to be improved so that it properly credits
> >everyone. However I think I have the major stroeks correct.
> >
> >From: "Eric W. Biederman" <ebiederm@xmission.com>
> >Date: Tue, 23 Jul 2019 07:44:46 -0500
> >Subject: [PATCH] waitid: Add support for waiting for the current
> >process group
> >
> >It was recently discovered that the linux version of waitid is not a
> >superset of the other wait functions because it does not include
> >support for waiting for the current process group. This has two
> >downsides. An extra system call is needed to get the current process
> >group, and a signal could come in between the system call that
> >retrieved the process gorup and the call to waitid that changes the
> >current process group.
> >
> >Allow userspace to avoid both of those issues by defining
> >idtype == P_PGID and id == 0 to mean wait for the caller's process
> >group at the time of the call.
> >
> >Arguments can be made for using a different choice of idtype and id
> >for this case but the BSDs already use this P_PGID and 0 to indicate
> >waiting for the current process's process group. So be nice to user
> >space programmers and don't introduce an unnecessary incompatibility.
> >
> >Some people have noted that the posix description is that
> >waitpid will wait for the current process group, and that in
> >the presence of pthreads that process group can change. To get
> >clarity on this issue I looked at XNU, FreeBSD, and Luminos. All of
> >those flavors of unix waited for the current process group at the
> >time of call and as written could not adapt to the process group
> >changing after the call.
> >
> >At one point Linux did adapt to the current process group changing but
> >that stopped in 161550d74c07 ("pid: sys_wait... fixes"). It has been
> >over 11 years since Linux has that behavior, no programs that fail
> >with the change in behavior have been reported, and I could not
> >find any other unix that does this. So I think it is safe to clarify
> >the definition of current process group, to current process group
> >at the time of the wait function.
> >
> >Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> >---
> > kernel/exit.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/kernel/exit.c b/kernel/exit.c
> >index a75b6a7f458a..3d86930f035e 100644
> >--- a/kernel/exit.c
> >+++ b/kernel/exit.c
> >@@ -1577,14 +1577,16 @@ static long kernel_waitid(int which, pid_t
> >upid, struct waitid_info *infop,
> > break;
> > case P_PGID:
> > type = PIDTYPE_PGID;
> >- if (upid <= 0)
> >+ if (upid < 0)
> > return -EINVAL;
> >+ if (upid == 0)
> >+ pid = get_pid(task_pgrp(current));
> > break;
> > default:
> > return -EINVAL;
> > }
> >
> >- if (type < PIDTYPE_MAX)
> >+ if ((type < PIDTYPE_MAX) && !pid)
> > pid = find_get_pid(upid);
> >
> > wo.wo_type = type;
>
> Eric, mind if I send this out alongside the P_PIDFD patchset and put in a test for it?
Was this ever sent?
Alistair
>
> Christian