LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Rich Felker <dalias@libc.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <christian.brauner@ubuntu.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
	GNU C Library <libc-alpha@sourceware.org>,
	Alistair Francis <alistair23@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	Florian Weimer <fweimer@redhat.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	macro@wdc.com, Zong Li <zongbox@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>, Peter Anvin <hpa@zytor.com>
Subject: Re: [PATCH v3 1/1] waitid: Add support for waiting for the current process group
Date: Wed, 14 Aug 2019 14:00:41 -0400
Message-ID: <20190814180041.GK9017@brightrain.aerifal.cx> (raw)
In-Reply-To: <CAHk-=wgpoeAnhscv9+fKNLLJF0tvypGPAxyzBCa0rp5hppRDRQ@mail.gmail.com>

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

  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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 [this message]
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

Reply instructions:

You may reply publically 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=20190814180041.GK9017@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=adhemerval.zanella@linaro.org \
    --cc=akpm@linux-foundation.org \
    --cc=alistair23@gmail.com \
    --cc=arnd@arndb.de \
    --cc=christian.brauner@ubuntu.com \
    --cc=ebiederm@xmission.com \
    --cc=fweimer@redhat.com \
    --cc=hpa@zytor.com \
    --cc=libc-alpha@sourceware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=macro@wdc.com \
    --cc=oleg@redhat.com \
    --cc=palmer@sifive.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zongbox@gmail.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git