public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Scott James Remnant <scott@canonical.com>
Cc: Roland McGrath <roland@redhat.com>, Ingo Molnar <mingo@elte.hu>,
	Casey Dahlin <cdahlin@redhat.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Randy Dunlap <randy.dunlap@oracle.com>,
	Davide Libenzi <davidel@xmailserver.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: Re: [RESEND][RFC PATCH v2] waitfd
Date: Sat, 10 Jan 2009 19:13:40 +0100	[thread overview]
Message-ID: <20090110181340.GA14978@redhat.com> (raw)
In-Reply-To: <1231607252.11642.103.camel@quest>

On 01/10, Scott James Remnant wrote:
>
> On Sat, 2009-01-10 at 16:57 +0100, Oleg Nesterov wrote:
>
> > I can't understand why should we change ->exit_signal if we want to
> > use signalfd. Yes, SIGCHLD is not rt. So what?
> >
> > We do not need multiple signals in queue if we want to reap multiple
> > zombies. Once we have a single SIGCHLD (reported by signalfd or
> > whatever) we can do do_wait(WNOHANG) in a loop.
> >
> Well, a good reason why is that it makes things much easier to do in
> userspace.

I never argued with this. And, let me repeat. I am not arguing against
waitfd! Actually, I always try to avoid the "do we need this feature"
discussions.

What I disagree with is that waitfd adds the functionality which does
not exists currently.

> You may as well ask why we have signalfd() at all, and what was wrong
> with sigaction() and ordinary signal handlers?  Well, lots of things

Cough. You don't have to explain me why signalfd is nice ;) I participated
in discussion when it was created.

> So let's compare userspace code for trying to reap children using
> signalfd();
>
> First, what we have today:
>
> [...snip the code...]
>
> Pros:
>  - code exists today

That is what I meant. Not more.

> Cons:
>  - having siginfo_t returned by read() is pointless, we can't use it

Indeed. We use read() only to wait for the signal death.

>  - double loop isn't pretty

Nice argument to add the new syscall ;)

>  - strange waitid() API in case of WNOHANG and no child

Heh. I also don't like this ;) A reason for waitfd ?

>  - incompatible structures for signalfd()'s read result and waitid(),
>    despite being logically the same structure! :-/

I could blaim waitfd because it fills siginfo in the manner which
is not compatible with signalfd, despite logically the same structure.

>  - can't simultaneously clear pending signal and wait, so we always have
>    to go back round the main loop if a child dies after the read()

Can't understand... waitfd doesn't clear the signal too?

And you forget to mention another drwaback with the current code:
a lot of pathetic comments ;)

> Since there's no point listening to SIGCHLD, it's a complete no-op, we
> don't respond to it at all.  We only need to use it to wake up the main
> loop.

Yes, sure, indeed, of course.

> The wait() loop tends to be at the bottom of the main loop
> somewhere, completely outside of the fd processing.

Huh.

> Now, what if signalfd() would always queue pending signals even if
> they're non-RT?

Well, I think this is off-topic, and more importantly I don't think
this change is possible.

> So what about
> waitfd()

Yes, the user-space code (for this particular artificial example)
becomes simpler. Following this logic, let's add sys_copyfile()
to kernel? From time to time I regret we don't have it...

(from another thread)
> > I am not sure we are talking about the same thing, but afaics poll() +
> > signalfd can work to (say) reap the childs. Actually, ppoll() alone is
> > enough.
> >
> Last time I checked, ppoll() was not actually implemented across all
> architectures in a manner that solved the race it was intended to solve.
>
> I'd be delighted to learn that this had been fixed? :-)

Scott, this is unfair. Yes, some arches do not implement restore_sigmask()
logic. So what? Let's suppose ppoll() has a bug. So, this means we should
add waitfd? No, let's fix ppol(), and waitfd is orthogonal. Imho.


Again, again, again. Please don't forget about "I am not arguing against".
But I don't buy your arguments.

Oleg.


  reply	other threads:[~2009-01-10 18:17 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-06 18:11 [RFC PATCH v2] waitfd Casey Dahlin
2009-01-06 18:27 ` Alan Cox
2009-01-06 18:31 ` Randy Dunlap
2009-01-06 18:45   ` Casey Dahlin
2009-01-06 18:50     ` Randy Dunlap
2009-01-06 18:48 ` Andi Kleen
2009-01-06 19:07 ` [RESEND][RFC " Casey Dahlin
2009-01-07 12:34   ` Ingo Molnar
2009-01-07 13:05     ` Casey Dahlin
2009-01-07 15:00       ` Ingo Molnar
2009-01-07 17:19     ` Oleg Nesterov
2009-01-07 17:24       ` Ingo Molnar
2009-01-07 17:52       ` Davide Libenzi
2009-01-07 20:38         ` Casey Dahlin
2009-01-10 14:47       ` Scott James Remnant
2009-01-10 21:14         ` Casey Dahlin
2009-01-10 21:20           ` Scott James Remnant
2009-01-10 22:08             ` Casey Dahlin
2009-01-10 22:31           ` Oleg Nesterov
2009-01-10 22:37             ` Casey Dahlin
2009-01-10 22:46               ` Oleg Nesterov
2009-01-07 20:53     ` Roland McGrath
2009-01-07 20:58       ` Ingo Molnar
2009-01-07 21:05         ` Davide Libenzi
2009-01-07 21:50           ` Ingo Molnar
2009-01-07 21:02       ` Ulrich Drepper
2009-01-08 14:32         ` Oleg Nesterov
2009-01-08 19:35           ` Roland McGrath
2009-01-08 20:36             ` Casey Dahlin
2009-01-08 21:39               ` Oleg Nesterov
2009-01-10 14:52                 ` Scott James Remnant
2009-01-10 16:19                   ` Oleg Nesterov
2009-01-10 17:09                     ` Scott James Remnant
2009-01-10 18:21                       ` Oleg Nesterov
2009-01-10 18:46                         ` Scott James Remnant
2009-01-10 14:50               ` Scott James Remnant
2009-01-10 21:20                 ` Casey Dahlin
2009-01-08 22:04       ` Michael Kerrisk
2009-01-10 14:09       ` Scott James Remnant
2009-01-10 14:45       ` Scott James Remnant
2009-01-10 15:57         ` Oleg Nesterov
2009-01-10 17:07           ` Scott James Remnant
2009-01-10 18:13             ` Oleg Nesterov [this message]
2009-01-10 20:13               ` Scott James Remnant
2009-01-10 22:24                 ` Oleg Nesterov
2009-01-10 23:14                   ` Davide Libenzi
2009-01-10 22:25             ` Casey Dahlin
2009-01-10 23:11             ` Davide Libenzi
2011-03-02  1:37           ` Denys Vlasenko
2011-03-02 13:55             ` Oleg Nesterov

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090110181340.GA14978@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=cdahlin@redhat.com \
    --cc=davidel@xmailserver.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=randy.dunlap@oracle.com \
    --cc=roland@redhat.com \
    --cc=scott@canonical.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
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