From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-2.8 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_GMAIL_RCVD, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 4A1D61F45A for ; Tue, 13 Aug 2019 22:26:01 +0000 (UTC) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; q=dns; s=default; b=ZksL XH0sKoQxh493YsqW0OtH/Qpkwnib+0FFTHYXYYVtFmP0t7p23gO6dV8MRdBKaNgL 7I+h2zqcvpr/o5jw2E85eIEc9sKQQEU1uRKUMCf24jmkbNpCjH9dLPaDVJ8pp6eX kXoGNISFm1pTFNDEkZe0zTWrg3oUE0Hm8gbMbNI= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-type; s=default; bh=onPG4n7w8j 7hBUXcjUgPFm0BVZg=; b=amT7+SBJmX8suGUWiYnqmlv8tfMqy9Mb9LaDhM8nG0 gqnTn12VRjgZA2nOzhDo+/Tf0urCIMEhBlIcXBb05f6HWuoSpEbuWrYYO+MbSzkk MMn9wWQr+LXArBme7xyw/5uZ/vgpqt5h1qoad1RdV/clnfhEwJWgcP9+nKM1lCkl w= Received: (qmail 63347 invoked by alias); 13 Aug 2019 22:25:58 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 63337 invoked by uid 89); 13 Aug 2019 22:25:58 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: mail-lf1-f68.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+t5exTpUo9Hbbm7PFSJAtvskDNLWYZm4hCCjnHcp81o=; b=IAzMqH8U8yFYI32Hn0F67mdQAP6C/yIAlJtK3cOmcxOlg4OCUPGC/mN0ZoALHBf+8b S5qnTvBAFknOwlNjujnq/20grH8qV6qngmie9pKuQkR3IIBoQC0Fg55Rww9zEJSfIiBx KAJQOmB9cfP0HrsBc3Fn5saPg2S07R/VtbK73E9xJkDTDxKInqM3x6SIe2ZL+Podoae9 VNCTIcgKSxS4rBfnyCSg2eSANwi0PVPb87jgTH6s9PB5OfhKE1pVvExH+QqB7j9P9/9K dJiX29DHUxaUjJHvwLFAQrhZfAXMjj2nMzI3WYjZDNnhp4Qj9b22JxanC/vpSOFl32I6 IFvg== MIME-Version: 1.0 References: <87muh79yt2.fsf@xmission.com> <87blxn83sk.fsf@xmission.com> <20190721232336.GA30851@brightrain.aerifal.cx> <87k1c962ml.fsf@xmission.com> <20190723082857.kf2go2vfvnu7q7zd@brauner.io> <20190725044009.GJ1506@brightrain.aerifal.cx> <8736iuujdu.fsf@xmission.com> <72291182-11E1-49C2-9608-875FD9473719@brauner.io> In-Reply-To: <72291182-11E1-49C2-9608-875FD9473719@brauner.io> From: Alistair Francis Date: Tue, 13 Aug 2019 15:22:02 -0700 Message-ID: Subject: Re: [RFC v3 03/23] sysdeps/wait: Use waitid if avaliable To: Christian Brauner Cc: "Eric W. Biederman" , Arnd Bergmann , Rich Felker , Linus Torvalds , Alistair Francis , GNU C Library , Adhemerval Zanella , Florian Weimer , Palmer Dabbelt , macro@wdc.com, Zong Li , Andrew Morton , Al Viro , "H. Peter Anvin" Content-Type: text/plain; charset="UTF-8" On Thu, Jul 25, 2019 at 10:30 AM Christian Brauner wrote: > > On July 25, 2019 7:14:05 PM GMT+02:00, ebiederm@xmission.com wrote: > >Arnd Bergmann writes: > > > >> On Thu, Jul 25, 2019 at 6:40 AM Rich Felker 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 > >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" > >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" > >--- > > 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