All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.