* [PATCH v1 0/1] waitid: process group enhancement [not found] <CAKmqyKMJPQAOKn11xepzAwXOd4e9dU0Cyz=A0T-uMEgUp5yJjA@mail.gmail.com> @ 2019-08-14 11:38 ` christian.brauner 2019-08-14 11:38 ` [PATCH v1 1/1] waitid: Add support for waiting for the current process group christian.brauner 2019-08-14 13:07 ` [PATCH v2 0/1] waitid: process group enhancement Christian Brauner 2019-08-14 15:43 ` [PATCH v3 0/1] waitid: process group enhancement Christian Brauner 2 siblings, 1 reply; 24+ messages in thread From: christian.brauner @ 2019-08-14 11:38 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa, Christian Brauner From: Christian Brauner <christian.brauner@ubuntu.com> Hey everyone, This patch adds support for waiting on the current process group by specifying waitid(P_PGID, 0, ...) as discussed in [1]. The details why we need to do this are in the commit message of [PATCH 1/1] so I won't repeat them here. I've picked this up since the thread has gone stale and parts of userspace are actually blocked by this. Note that the patch has been marked with v1 because I've changed the patch to be more closely aligned with the P_PIDFD changes to waitid() I have sitting in my for-next branch (cf. [2]). This makes the merge conflict a little simpler and picks up on the coding style discussions that guided the P_PIDFD patchset. There was some desire to get this feature in with 5.3 (cf. [3]). But given that this is a new feature for waitid() and for the sake of avoiding any merge conflicts I would prefer to land this in the 5.4 merge window together with the P_PIDFD changes. Thanks! Christian /* References */ [1]: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00587.html [2]: https://lore.kernel.org/lkml/20190727222229.6516-1-christian@brauner.io/ [3]: https://www.sourceware.org/ml/libc-alpha/2019-08/msg00304.html Eric W. Biederman (1): waitid: Add support for waiting for the current process group kernel/exit.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v1 1/1] waitid: Add support for waiting for the current process group 2019-08-14 11:38 ` [PATCH v1 0/1] waitid: process group enhancement christian.brauner @ 2019-08-14 11:38 ` christian.brauner 2019-08-14 12:29 ` Oleg Nesterov 0 siblings, 1 reply; 24+ messages in thread From: christian.brauner @ 2019-08-14 11:38 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa From: "Eric W. Biederman" <ebiederm@xmission.com> 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> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Palmer Dabbelt <palmer@sifive.com> Cc: Rich Felker <dalias@libc.org> Cc: Alistair Francis <alistair23@gmail.com> Cc: Zong Li <zongbox@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Florian Weimer <fweimer@redhat.com> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org> Cc: GNU C Library <libc-alpha@sourceware.org> --- v1: - Christian Brauner <christian.brauner@ubuntu.com>: - move find_get_pid() calls into the switch statements to minimize merge conflicts with P_PIDFD changes and adhere to coding style discussions we had for P_PIDFD --- kernel/exit.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 5b4a5dcce8f8..e70083b14f31 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -1576,19 +1576,23 @@ static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop, type = PIDTYPE_PID; if (upid <= 0) return -EINVAL; + + pid = find_get_pid(upid); break; case P_PGID: type = PIDTYPE_PGID; - if (upid <= 0) + if (upid < 0) return -EINVAL; + + if (upid == 0) + pid = get_pid(task_pgrp(current)); + else + pid = find_get_pid(upid); break; default: return -EINVAL; } - if (type < PIDTYPE_MAX) - pid = find_get_pid(upid); - wo.wo_type = type; wo.wo_pid = pid; wo.wo_flags = options; -- 2.22.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/1] waitid: Add support for waiting for the current process group 2019-08-14 11:38 ` [PATCH v1 1/1] waitid: Add support for waiting for the current process group christian.brauner @ 2019-08-14 12:29 ` Oleg Nesterov 2019-08-14 12:45 ` Christian Brauner 0 siblings, 1 reply; 24+ messages in thread From: Oleg Nesterov @ 2019-08-14 12:29 UTC (permalink / raw) To: christian.brauner Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On 08/14, christian.brauner@ubuntu.com wrote: > > case P_PGID: > type = PIDTYPE_PGID; > - if (upid <= 0) > + if (upid < 0) > return -EINVAL; > + > + if (upid == 0) > + pid = get_pid(task_pgrp(current)); this needs rcu lock or tasklist_lock, this can race with another thread doing sys_setpgid/setsid (see change_pid(PIDTYPE_PGID)). Oleg. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/1] waitid: Add support for waiting for the current process group 2019-08-14 12:29 ` Oleg Nesterov @ 2019-08-14 12:45 ` Christian Brauner 2019-08-14 12:50 ` Oleg Nesterov 0 siblings, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 12:45 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 02:29:10PM +0200, Oleg Nesterov wrote: > On 08/14, christian.brauner@ubuntu.com wrote: > > > > case P_PGID: > > type = PIDTYPE_PGID; > > - if (upid <= 0) > > + if (upid < 0) > > return -EINVAL; > > + > > + if (upid == 0) > > + pid = get_pid(task_pgrp(current)); > > this needs rcu lock or tasklist_lock, this can race with another thread > doing sys_setpgid/setsid (see change_pid(PIDTYPE_PGID)). Oh, I naively assumed task_pgrp() would take an rcu lock... kernel/sys.c takes both, i.e. rcu_read_lock(); write_lock_irq(&tasklist_lock); though I think we should be fine with just rcu_read_lock(). setpgid() indicates that it wants to check real_parent and needs the write_lock_irq() because it might change behind its back which we don't care about since we're not changing the pgrp. Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/1] waitid: Add support for waiting for the current process group 2019-08-14 12:45 ` Christian Brauner @ 2019-08-14 12:50 ` Oleg Nesterov 2019-08-14 12:53 ` Christian Brauner 0 siblings, 1 reply; 24+ messages in thread From: Oleg Nesterov @ 2019-08-14 12:50 UTC (permalink / raw) To: Christian Brauner Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On 08/14, Christian Brauner wrote: > > On Wed, Aug 14, 2019 at 02:29:10PM +0200, Oleg Nesterov wrote: > > On 08/14, christian.brauner@ubuntu.com wrote: > > > > > > case P_PGID: > > > type = PIDTYPE_PGID; > > > - if (upid <= 0) > > > + if (upid < 0) > > > return -EINVAL; > > > + > > > + if (upid == 0) > > > + pid = get_pid(task_pgrp(current)); > > > > this needs rcu lock or tasklist_lock, this can race with another thread > > doing sys_setpgid/setsid (see change_pid(PIDTYPE_PGID)). > > Oh, I naively assumed task_pgrp() would take an rcu lock... but it would not help ;) > though I think we should be fine with just rcu_read_lock(). Yes, Oleg. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v1 1/1] waitid: Add support for waiting for the current process group 2019-08-14 12:50 ` Oleg Nesterov @ 2019-08-14 12:53 ` Christian Brauner 0 siblings, 0 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 12:53 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 02:50:12PM +0200, Oleg Nesterov wrote: > On 08/14, Christian Brauner wrote: > > > > On Wed, Aug 14, 2019 at 02:29:10PM +0200, Oleg Nesterov wrote: > > > On 08/14, christian.brauner@ubuntu.com wrote: > > > > > > > > case P_PGID: > > > > type = PIDTYPE_PGID; > > > > - if (upid <= 0) > > > > + if (upid < 0) > > > > return -EINVAL; > > > > + > > > > + if (upid == 0) > > > > + pid = get_pid(task_pgrp(current)); > > > > > > this needs rcu lock or tasklist_lock, this can race with another thread > > > doing sys_setpgid/setsid (see change_pid(PIDTYPE_PGID)). > > > > Oh, I naively assumed task_pgrp() would take an rcu lock... > > but it would not help ;) Yeah, it doesn't do a get. :) ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/1] waitid: process group enhancement [not found] <CAKmqyKMJPQAOKn11xepzAwXOd4e9dU0Cyz=A0T-uMEgUp5yJjA@mail.gmail.com> 2019-08-14 11:38 ` [PATCH v1 0/1] waitid: process group enhancement christian.brauner @ 2019-08-14 13:07 ` Christian Brauner 2019-08-14 13:07 ` [PATCH v2 1/1] waitid: Add support for waiting for the current process group Christian Brauner 2019-08-14 15:43 ` [PATCH v3 0/1] waitid: process group enhancement Christian Brauner 2 siblings, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 13:07 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa, Christian Brauner Hey everyone, This patch adds support for waiting on the current process group by specifying waitid(P_PGID, 0, ...) as discussed in [1]. The details why we need to do this are in the commit message of [PATCH 1/1] so I won't repeat them here. I've picked this up since the thread has gone stale and parts of userspace are actually blocked by this. Note that the patch has been changed to be more closely aligned with the P_PIDFD changes to waitid() I have sitting in my for-next branch (cf. [2]). This makes the merge conflict a little simpler and picks up on the coding style discussions that guided the P_PIDFD patchset. There was some desire to get this feature in with 5.3 (cf. [3]). But given that this is a new feature for waitid() and for the sake of avoiding any merge conflicts I would prefer to land this in the 5.4 merge window together with the P_PIDFD changes. Thanks! Christian /* References */ [1]: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00587.html [2]: https://lore.kernel.org/lkml/20190727222229.6516-1-christian@brauner.io/ [3]: https://www.sourceware.org/ml/libc-alpha/2019-08/msg00304.html Eric W. Biederman (1): waitid: Add support for waiting for the current process group kernel/exit.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/1] waitid: Add support for waiting for the current process group 2019-08-14 13:07 ` [PATCH v2 0/1] waitid: process group enhancement Christian Brauner @ 2019-08-14 13:07 ` Christian Brauner 2019-08-14 14:19 ` Oleg Nesterov 0 siblings, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 13:07 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa, Christian Brauner From: "Eric W. Biederman" <ebiederm@xmission.com> 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> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Palmer Dabbelt <palmer@sifive.com> Cc: Rich Felker <dalias@libc.org> Cc: Alistair Francis <alistair23@gmail.com> Cc: Zong Li <zongbox@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Florian Weimer <fweimer@redhat.com> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org> Cc: GNU C Library <libc-alpha@sourceware.org> --- v1: - Christian Brauner <christian.brauner@ubuntu.com>: - move find_get_pid() calls into the switch statements to minimize merge conflicts with P_PIDFD changes and adhere to coding style discussions we had for P_PIDFD v2: - Oleg Nesterov <oleg@redhat.com>: - take rcu_read_lock() around task_pgrp() so that we don't race with another thread changing the pgrp - Christian Brauner <christian.brauner@ubuntu.com>: - introduce find_get_pgrp() --- kernel/exit.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 5b4a5dcce8f8..d02715a39f68 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -1554,6 +1554,20 @@ static long do_wait(struct wait_opts *wo) return retval; } +static struct pid *find_get_pgrp(pid_t nr) +{ + struct pid *pid; + + if (nr) + return find_get_pid(nr); + + rcu_read_lock(); + pid = get_pid(task_pgrp(current)); + rcu_read_unlock(); + + return pid; +} + static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop, int options, struct rusage *ru) { @@ -1576,19 +1590,20 @@ static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop, type = PIDTYPE_PID; if (upid <= 0) return -EINVAL; + + pid = find_get_pid(upid); break; case P_PGID: type = PIDTYPE_PGID; - if (upid <= 0) + if (upid < 0) return -EINVAL; + + pid = find_get_pgrp(upid); break; default: return -EINVAL; } - if (type < PIDTYPE_MAX) - pid = find_get_pid(upid); - wo.wo_type = type; wo.wo_pid = pid; wo.wo_flags = options; -- 2.22.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/1] waitid: Add support for waiting for the current process group 2019-08-14 13:07 ` [PATCH v2 1/1] waitid: Add support for waiting for the current process group Christian Brauner @ 2019-08-14 14:19 ` Oleg Nesterov 2019-08-14 14:35 ` Christian Brauner 0 siblings, 1 reply; 24+ messages in thread From: Oleg Nesterov @ 2019-08-14 14:19 UTC (permalink / raw) To: Christian Brauner Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On 08/14, Christian Brauner wrote: > > +static struct pid *find_get_pgrp(pid_t nr) > +{ > + struct pid *pid; > + > + if (nr) > + return find_get_pid(nr); > + > + rcu_read_lock(); > + pid = get_pid(task_pgrp(current)); > + rcu_read_unlock(); > + > + return pid; > +} I can't say I like this helper... even its name doesn't look good to me. I forgot that we already have get_task_pid() when I replied to the previous version... How about case P_PGID: if (upid) pid = find_get_pid(upid); else pid = get_task_pid(current, PIDTYPE_PGID); ? Oleg. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/1] waitid: Add support for waiting for the current process group 2019-08-14 14:19 ` Oleg Nesterov @ 2019-08-14 14:35 ` Christian Brauner 2019-08-14 15:27 ` Oleg Nesterov 0 siblings, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 14:35 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 04:19:57PM +0200, Oleg Nesterov wrote: > On 08/14, Christian Brauner wrote: > > > > +static struct pid *find_get_pgrp(pid_t nr) > > +{ > > + struct pid *pid; > > + > > + if (nr) > > + return find_get_pid(nr); > > + > > + rcu_read_lock(); > > + pid = get_pid(task_pgrp(current)); > > + rcu_read_unlock(); > > + > > + return pid; > > +} > > I can't say I like this helper... even its name doesn't look good to me. Well, naming scheme obviously stolen from find_get_pid(). Not sure if that doesn't look good as well. ;) > > I forgot that we already have get_task_pid() when I replied to the previous > version... How about > > case P_PGID: > > if (upid) > pid = find_get_pid(upid); > else > pid = get_task_pid(current, PIDTYPE_PGID); > > ? Hmyeah, that works but wouldn't it still be nicer to simply have: static struct pid *get_pgrp(pid_t nr) { if (nr) return find_get_pid(nr); return get_task_pid(current, PIDTYPE_PGID); } That allows us to have all cases equivalent with a single call to a function without any if-elses. Imho, this becomes especially nice when considering that P_PIDFD goes on top of that: switch (which) { case P_ALL: type = PIDTYPE_MAX; break; case P_PID: type = PIDTYPE_PID; if (upid <= 0) return -EINVAL; pid = find_get_pid(upid); break; case P_PGID: type = PIDTYPE_PGID; if (upid < 0) return -EINVAL; pid = find_get_pgrp(upid); break; case P_PIDFD: type = PIDTYPE_PID; if (upid < 0) return -EINVAL; pid = pidfd_get_pid(upid); if (IS_ERR(pid)) return PTR_ERR(pid); break; default: return -EINVAL; } Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/1] waitid: Add support for waiting for the current process group 2019-08-14 14:35 ` Christian Brauner @ 2019-08-14 15:27 ` Oleg Nesterov 2019-08-14 15:30 ` Christian Brauner 0 siblings, 1 reply; 24+ messages in thread From: Oleg Nesterov @ 2019-08-14 15:27 UTC (permalink / raw) To: Christian Brauner Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On 08/14, Christian Brauner wrote: > > On Wed, Aug 14, 2019 at 04:19:57PM +0200, Oleg Nesterov wrote: > > On 08/14, Christian Brauner wrote: > > > > > > +static struct pid *find_get_pgrp(pid_t nr) > > > +{ > > > + struct pid *pid; > > > + > > > + if (nr) > > > + return find_get_pid(nr); > > > + > > > + rcu_read_lock(); > > > + pid = get_pid(task_pgrp(current)); > > > + rcu_read_unlock(); > > > + > > > + return pid; > > > +} > > > > I can't say I like this helper... even its name doesn't look good to me. > > Well, naming scheme obviously stolen from find_get_pid(). Not sure if > that doesn't look good as well. ;) find_get_pid() actually tries to find a pid. The helper above does "find" or "use current" depending on nr != 0. > > I forgot that we already have get_task_pid() when I replied to the previous > > version... How about > > > > case P_PGID: > > > > if (upid) > > pid = find_get_pid(upid); > > else > > pid = get_task_pid(current, PIDTYPE_PGID); > > > > ? > > Hmyeah, that works but wouldn't it still be nicer to simply have: > > static struct pid *get_pgrp(pid_t nr) > { > if (nr) > return find_get_pid(nr); > > return get_task_pid(current, PIDTYPE_PGID); > } Who else can ever use it? It saves 4 lines in kernel_waitid() but adds 7 lines outside, and you need another ^] to see these lines if you try to understand what PIDTYPE_PGID actually does. IOW, I think this helper will make waitid less readable for no reason. Christian, I try to never argue when it comes to cosmetic issues, and in this case I won't insist too. Oleg. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/1] waitid: Add support for waiting for the current process group 2019-08-14 15:27 ` Oleg Nesterov @ 2019-08-14 15:30 ` Christian Brauner 0 siblings, 0 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 15:30 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 05:27:12PM +0200, Oleg Nesterov wrote: > On 08/14, Christian Brauner wrote: > > > > On Wed, Aug 14, 2019 at 04:19:57PM +0200, Oleg Nesterov wrote: > > > On 08/14, Christian Brauner wrote: > > > > > > > > +static struct pid *find_get_pgrp(pid_t nr) > > > > +{ > > > > + struct pid *pid; > > > > + > > > > + if (nr) > > > > + return find_get_pid(nr); > > > > + > > > > + rcu_read_lock(); > > > > + pid = get_pid(task_pgrp(current)); > > > > + rcu_read_unlock(); > > > > + > > > > + return pid; > > > > +} > > > > > > I can't say I like this helper... even its name doesn't look good to me. > > > > Well, naming scheme obviously stolen from find_get_pid(). Not sure if > > that doesn't look good as well. ;) > > find_get_pid() actually tries to find a pid. The helper above does "find" > or "use current" depending on nr != 0. > > > > I forgot that we already have get_task_pid() when I replied to the previous > > > version... How about > > > > > > case P_PGID: > > > > > > if (upid) > > > pid = find_get_pid(upid); > > > else > > > pid = get_task_pid(current, PIDTYPE_PGID); > > > > > > ? > > > > Hmyeah, that works but wouldn't it still be nicer to simply have: > > > > static struct pid *get_pgrp(pid_t nr) > > { > > if (nr) > > return find_get_pid(nr); > > > > return get_task_pid(current, PIDTYPE_PGID); > > } > > Who else can ever use it? > > It saves 4 lines in kernel_waitid() but adds 7 lines outside, and you > need another ^] to see these lines if you try to understand what > PIDTYPE_PGID actually does. IOW, I think this helper will make waitid > less readable for no reason. > > > Christian, I try to never argue when it comes to cosmetic issues, and > in this case I won't insist too. Yeah, I know. I'm not insisisting either. We can do your thing since you do after all seem to care at least a tiny bit. ;) Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 0/1] waitid: process group enhancement [not found] <CAKmqyKMJPQAOKn11xepzAwXOd4e9dU0Cyz=A0T-uMEgUp5yJjA@mail.gmail.com> 2019-08-14 11:38 ` [PATCH v1 0/1] waitid: process group enhancement christian.brauner 2019-08-14 13:07 ` [PATCH v2 0/1] waitid: process group enhancement Christian Brauner @ 2019-08-14 15:43 ` Christian Brauner 2019-08-14 15:44 ` [PATCH v3 1/1] waitid: Add support for waiting for the current process group Christian Brauner 2019-08-14 15:58 ` [PATCH v3 0/1] waitid: process group enhancement Rich Felker 2 siblings, 2 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 15:43 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa, Christian Brauner Hey everyone, This patch adds support for waiting on the current process group by specifying waitid(P_PGID, 0, ...) as discussed in [1]. The details why we need to do this are in the commit message of [PATCH 1/1] so I won't repeat them here. I've picked this up since the thread has gone stale and parts of userspace are actually blocked by this. Note that the patch has been changed to be more closely aligned with the P_PIDFD changes to waitid() I have sitting in my for-next branch (cf. [2]). This makes the merge conflict a little simpler and picks up on the coding style discussions that guided the P_PIDFD patchset. There was some desire to get this feature in with 5.3 (cf. [3]). But given that this is a new feature for waitid() and for the sake of avoiding any merge conflicts I would prefer to land this in the 5.4 merge window together with the P_PIDFD changes. Thanks! Christian /* v0 */ Link: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00587.html /* v1 */ Link: https://lore.kernel.org/lkml/20190814113822.9505-1-christian.brauner@ubuntu.com/ /* v2 */ Link: https://lore.kernel.org/lkml/20190814130732.23572-1-christian.brauner@ubuntu.com/ /* References */ [1]: https://www.sourceware.org/ml/libc-alpha/2019-07/msg00587.html [2]: https://lore.kernel.org/lkml/20190727222229.6516-1-christian@brauner.io/ [3]: https://www.sourceware.org/ml/libc-alpha/2019-08/msg00304.html Eric W. Biederman (1): waitid: Add support for waiting for the current process group kernel/exit.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) -- 2.22.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 15:43 ` [PATCH v3 0/1] waitid: process group enhancement Christian Brauner @ 2019-08-14 15:44 ` Christian Brauner 2019-08-14 16:09 ` Oleg Nesterov 2019-08-14 15:58 ` [PATCH v3 0/1] waitid: process group enhancement Rich Felker 1 sibling, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 15:44 UTC (permalink / raw) To: linux-kernel, libc-alpha Cc: oleg, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa, Christian Brauner From: "Eric W. Biederman" <ebiederm@xmission.com> 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> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Palmer Dabbelt <palmer@sifive.com> Cc: Rich Felker <dalias@libc.org> Cc: Alistair Francis <alistair23@gmail.com> Cc: Zong Li <zongbox@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Oleg Nesterov <oleg@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Florian Weimer <fweimer@redhat.com> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org> Cc: GNU C Library <libc-alpha@sourceware.org> --- v1: - Christian Brauner <christian.brauner@ubuntu.com>: - move find_get_pid() calls into the switch statements to minimize merge conflicts with P_PIDFD changes and adhere to coding style discussions we had for P_PIDFD v2: - Oleg Nesterov <oleg@redhat.com>: - take rcu_read_lock() around task_pgrp() so that we don't race with another thread changing the pgrp - Christian Brauner <christian.brauner@ubuntu.com>: - introduce find_get_pgrp() v3: - Oleg Nesterov <oleg@redhat.com>: - use get_task_pid() --- kernel/exit.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 5b4a5dcce8f8..8e4e357fddc8 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -1576,19 +1576,23 @@ static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop, type = PIDTYPE_PID; if (upid <= 0) return -EINVAL; + + pid = find_get_pid(upid); break; case P_PGID: type = PIDTYPE_PGID; - if (upid <= 0) + if (upid < 0) return -EINVAL; + + if (upid) + pid = find_get_pid(upid); + else + pid = get_task_pid(current, PIDTYPE_PGID); break; default: return -EINVAL; } - if (type < PIDTYPE_MAX) - pid = find_get_pid(upid); - wo.wo_type = type; wo.wo_pid = pid; wo.wo_flags = options; -- 2.22.0 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 15:44 ` [PATCH v3 1/1] waitid: Add support for waiting for the current process group Christian Brauner @ 2019-08-14 16:09 ` Oleg Nesterov 2019-08-14 16:15 ` Christian Brauner 2019-08-14 20:50 ` Christian Brauner 0 siblings, 2 replies; 24+ messages in thread From: Oleg Nesterov @ 2019-08-14 16:09 UTC (permalink / raw) To: Christian Brauner Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On 08/14, Christian Brauner wrote: > > 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. I noticed this typo only because I spent 2 minutes or more trying to understand this sentence ;) But yes, a signal handler or another thread can change pgrp in between. Reviewed-by: Oleg Nesterov <oleg@redhat.com> ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:09 ` Oleg Nesterov @ 2019-08-14 16:15 ` Christian Brauner 2019-08-14 16:34 ` Christian Brauner 2019-08-14 20:50 ` Christian Brauner 1 sibling, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 16:15 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 06:09:17PM +0200, Oleg Nesterov wrote: > On 08/14, Christian Brauner wrote: > > > > 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. > > I noticed this typo only because I spent 2 minutes or more trying to > understand this sentence ;) But yes, a signal handler or another thread I'll try to rewrite it. :) > can change pgrp in between. > > Reviewed-by: Oleg Nesterov <oleg@redhat.com> > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:15 ` Christian Brauner @ 2019-08-14 16:34 ` Christian Brauner 2019-08-14 16:55 ` Rich Felker 0 siblings, 1 reply; 24+ messages in thread From: Christian Brauner @ 2019-08-14 16:34 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 06:15:17PM +0200, Christian Brauner wrote: > On Wed, Aug 14, 2019 at 06:09:17PM +0200, Oleg Nesterov wrote: > > On 08/14, Christian Brauner wrote: > > > > > > 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. > > > > I noticed this typo only because I spent 2 minutes or more trying to > > understand this sentence ;) But yes, a signal handler or another thread > > I'll try to rewrite it. :) Ok, here's what I changed it to: 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: 1. An extra system call is needed to get the current process group. 2. After the current process group is received and before it is passed to waitid a signal could arrive causing the current process group to change. Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:34 ` Christian Brauner @ 2019-08-14 16:55 ` Rich Felker 2019-08-14 17:02 ` Christian Brauner 2019-08-14 17:06 ` Linus Torvalds 0 siblings, 2 replies; 24+ messages in thread From: Rich Felker @ 2019-08-14 16:55 UTC (permalink / raw) To: Christian Brauner Cc: Oleg Nesterov, linux-kernel, libc-alpha, alistair23, ebiederm, arnd, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 06:34:44PM +0200, Christian Brauner wrote: > On Wed, Aug 14, 2019 at 06:15:17PM +0200, Christian Brauner wrote: > > On Wed, Aug 14, 2019 at 06:09:17PM +0200, Oleg Nesterov wrote: > > > On 08/14, Christian Brauner wrote: > > > > > > > > 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. > > > > > > I noticed this typo only because I spent 2 minutes or more trying to > > > understand this sentence ;) But yes, a signal handler or another thread > > > > I'll try to rewrite it. :) > > Ok, here's what I changed it to: > > 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: > 1. An extra system call is needed to get the current process group. > 2. After the current process group is received and before it is passed > to waitid a signal could arrive causing the current process group to change. I don't think "downsides" sufficiently conveys that this is hard breakage of a requirement for waitpid. How about something like the following? "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. Userspace cannot simply emulate this functionality with an additional getpgid syscall due to inherent race conditions that violate the async-signal safety requirements for waitpid." Rich ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:55 ` Rich Felker @ 2019-08-14 17:02 ` Christian Brauner 2019-08-14 17:06 ` Linus Torvalds 1 sibling, 0 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 17:02 UTC (permalink / raw) To: Rich Felker Cc: Oleg Nesterov, linux-kernel, libc-alpha, alistair23, ebiederm, arnd, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 12:55:01PM -0400, Rich Felker wrote: > On Wed, Aug 14, 2019 at 06:34:44PM +0200, Christian Brauner wrote: > > On Wed, Aug 14, 2019 at 06:15:17PM +0200, Christian Brauner wrote: > > > On Wed, Aug 14, 2019 at 06:09:17PM +0200, Oleg Nesterov wrote: > > > > On 08/14, Christian Brauner wrote: > > > > > > > > > > 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. > > > > > > > > I noticed this typo only because I spent 2 minutes or more trying to > > > > understand this sentence ;) But yes, a signal handler or another thread > > > > > > I'll try to rewrite it. :) > > > > Ok, here's what I changed it to: > > > > 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: > > 1. An extra system call is needed to get the current process group. > > 2. After the current process group is received and before it is passed > > to waitid a signal could arrive causing the current process group to change. > > I don't think "downsides" sufficiently conveys that this is hard > breakage of a requirement for waitpid. How about something like the > following? > > "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. Userspace cannot > simply emulate this functionality with an additional getpgid syscall > due to inherent race conditions that violate the async-signal safety > requirements for waitpid." I like the rather specific example in there. How about we add that after this section like so: 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: 1. An extra system call is needed to get the current process group. 2. After the current process group is received and before it is passed to waitid a signal could arrive causing the current process group to change. Such inherent race-conditions as mentioned in 2. make it impossible for userspace to emulate this functionaly and thus violate the async-signal safety requirements for waitpid. Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:55 ` Rich Felker 2019-08-14 17:02 ` Christian Brauner @ 2019-08-14 17:06 ` Linus Torvalds 2019-08-14 18:00 ` Rich Felker 1 sibling, 1 reply; 24+ messages in thread From: Linus Torvalds @ 2019-08-14 17:06 UTC (permalink / raw) To: Rich Felker Cc: Christian Brauner, Oleg Nesterov, Linux List Kernel Mailing, GNU C Library, Alistair Francis, Eric W. Biederman, Arnd Bergmann, Adhemerval Zanella, Florian Weimer, Palmer Dabbelt, macro, Zong Li, Andrew Morton, Al Viro, Peter Anvin On Wed, Aug 14, 2019 at 9:55 AM Rich Felker <dalias@libc.org> wrote: > > I don't think "downsides" sufficiently conveys that this is hard > breakage of a requirement for waitpid. Well, let's be honest here. Who has _ever_ seen a signal handler changing the current process group? In fact, the SYSV version of setpgid() takes a process ID to set it *for somebody else*, so the signal safety is not even necessarily relevant, since it might be racing with _another_ thread doing it (which even the kernel side won't fix - it's just user space doing odd things). So yes - it's technically true that it's impossible to emulate properly in user space. But I doubt it makes _any_ difference what-so-ever, and glibc might as well do something like ret = waitid(P_PGID, 0, ..); if (ret == -EINVAL) { do the emulation } which makes it work with older kernels, and has zero downside in practice. Hmm? Linus ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 17:06 ` Linus Torvalds @ 2019-08-14 18:00 ` Rich Felker 0 siblings, 0 replies; 24+ messages in thread From: Rich Felker @ 2019-08-14 18:00 UTC (permalink / raw) To: Linus Torvalds Cc: Christian Brauner, Oleg Nesterov, Linux List Kernel Mailing, GNU C Library, Alistair Francis, Eric W. Biederman, Arnd Bergmann, Adhemerval Zanella, Florian Weimer, Palmer Dabbelt, macro, Zong Li, Andrew Morton, Al Viro, Peter Anvin On Wed, Aug 14, 2019 at 10:06:19AM -0700, Linus Torvalds wrote: > On Wed, Aug 14, 2019 at 9:55 AM Rich Felker <dalias@libc.org> wrote: > > > > I don't think "downsides" sufficiently conveys that this is hard > > breakage of a requirement for waitpid. > > Well, let's be honest here. Who has _ever_ seen a signal handler > changing the current process group? > > In fact, the SYSV version of setpgid() takes a process ID to set it > *for somebody else*, so the signal safety is not even necessarily > relevant, since it might be racing with _another_ thread doing it > (which even the kernel side won't fix - it's just user space doing odd > things). For that case, the operations are inherently unordered with respect to each other, and assuming the interpretation that waitpid is allowed to wait on "the pgid at the time of the call" rather than at the time of child exit/status-change -- which was discussed thoroughly in the thread leading up to this patch -- there is no conformance distinction. On the other hand, with changing your own pgid from a signal handler, there is a clear observable ordering between the events. For example, if the signal handler changes the pgid and forks a child with the new pgid, waitpid for "own pgid" can be assumed to include the new child in its wait set. I agree this is not common usage, so impact of breakage is probably low, but I'd rather not have wrong/racy hacks be something we're committed to supporting indefinitely on the userspace side. > So yes - it's technically true that it's impossible to emulate > properly in user space. > > But I doubt it makes _any_ difference what-so-ever, and glibc might as > well do something like > > ret = waitid(P_PGID, 0, ..); > if (ret == -EINVAL) { do the emulation } > > which makes it work with older kernels, and has zero downside in practice. > > Hmm? It only affects RV32 anyway; other archs all have a waitpid syscall that can be used. Since there's not yet any official libc release with RV32 support and AIUI the ABI is not considered "frozen" yet, emulation doesn't seem useful here. Whatever kernel version fixes this (or some later one, if nobody gets things together on upstreaming libc support of RV32) will just become the minimum version for RV32. Rich ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group 2019-08-14 16:09 ` Oleg Nesterov 2019-08-14 16:15 ` Christian Brauner @ 2019-08-14 20:50 ` Christian Brauner 1 sibling, 0 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 20:50 UTC (permalink / raw) To: Oleg Nesterov Cc: linux-kernel, libc-alpha, alistair23, ebiederm, arnd, dalias, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 06:09:17PM +0200, Oleg Nesterov wrote: > On 08/14, Christian Brauner wrote: > > > > 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. > > I noticed this typo only because I spent 2 minutes or more trying to > understand this sentence ;) But yes, a signal handler or another thread > can change pgrp in between. > > Reviewed-by: Oleg Nesterov <oleg@redhat.com> Applied-to: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=pidfd on top of P_PIDFD changes (cf. [1]) with merge conflict resolved (cf. [2]). (All changes on top of v5.3-rc1.) Merged-into: https://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git/log/?h=for-next and should show up in linux-next tomorrow. Thanks! Christian /* References */ [1]: https://lore.kernel.org/r/20190727222229.6516-2-christian@brauner.io [2]: patch after resolved merge-conflict: kernel/exit.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/kernel/exit.c b/kernel/exit.c index 64bb6893a37d..d2d74a7b81d1 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -1596,10 +1596,13 @@ 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; - pid = find_get_pid(upid); + if (upid) + pid = find_get_pid(upid); + else + pid = get_task_pid(current, PIDTYPE_PGID); break; case P_PIDFD: type = PIDTYPE_PID; ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/1] waitid: process group enhancement 2019-08-14 15:43 ` [PATCH v3 0/1] waitid: process group enhancement Christian Brauner 2019-08-14 15:44 ` [PATCH v3 1/1] waitid: Add support for waiting for the current process group Christian Brauner @ 2019-08-14 15:58 ` Rich Felker 2019-08-14 16:13 ` Christian Brauner 1 sibling, 1 reply; 24+ messages in thread From: Rich Felker @ 2019-08-14 15:58 UTC (permalink / raw) To: Christian Brauner Cc: linux-kernel, libc-alpha, oleg, alistair23, ebiederm, arnd, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 05:43:59PM +0200, Christian Brauner wrote: > Hey everyone, > > This patch adds support for waiting on the current process group by > specifying waitid(P_PGID, 0, ...) as discussed in [1]. The details why > we need to do this are in the commit message of [PATCH 1/1] so I won't > repeat them here. > > I've picked this up since the thread has gone stale and parts of > userspace are actually blocked by this. > > Note that the patch has been changed to be more closely aligned with the > P_PIDFD changes to waitid() I have sitting in my for-next branch (cf. [2]). > This makes the merge conflict a little simpler and picks up on the > coding style discussions that guided the P_PIDFD patchset. > > There was some desire to get this feature in with 5.3 (cf. [3]). > But given that this is a new feature for waitid() and for the sake of > avoiding any merge conflicts I would prefer to land this in the 5.4 > merge window together with the P_PIDFD changes. That makes 5.4 (or later, depending on other stuff) the hard minimum for RV32 ABI. Is that acceptable? I was under the impression (perhaps mistaken) that 5.3 was going to be next LTS series which is why I'd like to have the necessary syscalls for a complete working RV32 userspace in it. If I'm wrong about that please ignore me. :-) Rich ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/1] waitid: process group enhancement 2019-08-14 15:58 ` [PATCH v3 0/1] waitid: process group enhancement Rich Felker @ 2019-08-14 16:13 ` Christian Brauner 0 siblings, 0 replies; 24+ messages in thread From: Christian Brauner @ 2019-08-14 16:13 UTC (permalink / raw) To: Rich Felker Cc: linux-kernel, libc-alpha, oleg, alistair23, ebiederm, arnd, torvalds, adhemerval.zanella, fweimer, palmer, macro, zongbox, akpm, viro, hpa On Wed, Aug 14, 2019 at 11:58:22AM -0400, Rich Felker wrote: > On Wed, Aug 14, 2019 at 05:43:59PM +0200, Christian Brauner wrote: > > Hey everyone, > > > > This patch adds support for waiting on the current process group by > > specifying waitid(P_PGID, 0, ...) as discussed in [1]. The details why > > we need to do this are in the commit message of [PATCH 1/1] so I won't > > repeat them here. > > > > I've picked this up since the thread has gone stale and parts of > > userspace are actually blocked by this. > > > > Note that the patch has been changed to be more closely aligned with the > > P_PIDFD changes to waitid() I have sitting in my for-next branch (cf. [2]). > > This makes the merge conflict a little simpler and picks up on the > > coding style discussions that guided the P_PIDFD patchset. > > > > There was some desire to get this feature in with 5.3 (cf. [3]). > > But given that this is a new feature for waitid() and for the sake of > > avoiding any merge conflicts I would prefer to land this in the 5.4 > > merge window together with the P_PIDFD changes. > > That makes 5.4 (or later, depending on other stuff) the hard minimum > for RV32 ABI. Is that acceptable? I was under the impression (perhaps > mistaken) that 5.3 was going to be next LTS series which is why I'd > like to have the necessary syscalls for a complete working RV32 > userspace in it. If I'm wrong about that please ignore me. :-) 5.3 is not going to be an LTS and we don't do new features after the merge window is closed anyway. :) Christian ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-08-14 20:50 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CAKmqyKMJPQAOKn11xepzAwXOd4e9dU0Cyz=A0T-uMEgUp5yJjA@mail.gmail.com> 2019-08-14 11:38 ` [PATCH v1 0/1] waitid: process group enhancement christian.brauner 2019-08-14 11:38 ` [PATCH v1 1/1] waitid: Add support for waiting for the current process group christian.brauner 2019-08-14 12:29 ` Oleg Nesterov 2019-08-14 12:45 ` Christian Brauner 2019-08-14 12:50 ` Oleg Nesterov 2019-08-14 12:53 ` Christian Brauner 2019-08-14 13:07 ` [PATCH v2 0/1] waitid: process group enhancement Christian Brauner 2019-08-14 13:07 ` [PATCH v2 1/1] waitid: Add support for waiting for the current process group Christian Brauner 2019-08-14 14:19 ` Oleg Nesterov 2019-08-14 14:35 ` Christian Brauner 2019-08-14 15:27 ` Oleg Nesterov 2019-08-14 15:30 ` Christian Brauner 2019-08-14 15:43 ` [PATCH v3 0/1] waitid: process group enhancement Christian Brauner 2019-08-14 15:44 ` [PATCH v3 1/1] waitid: Add support for waiting for the current process group Christian Brauner 2019-08-14 16:09 ` Oleg Nesterov 2019-08-14 16:15 ` Christian Brauner 2019-08-14 16:34 ` Christian Brauner 2019-08-14 16:55 ` Rich Felker 2019-08-14 17:02 ` Christian Brauner 2019-08-14 17:06 ` Linus Torvalds 2019-08-14 18:00 ` Rich Felker 2019-08-14 20:50 ` Christian Brauner 2019-08-14 15:58 ` [PATCH v3 0/1] waitid: process group enhancement Rich Felker 2019-08-14 16:13 ` Christian Brauner
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).