All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joel Fernandes <joel@joelfernandes.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Christian Brauner <christian@brauner.io>,
	linux-kernel@vger.kernel.org, luto@amacapital.net,
	rostedt@goodmis.org, dancol@google.com, sspatil@google.com,
	jannh@google.com, surenb@google.com, timmurray@google.com,
	Jonathan Kowalski <bl0pbl33p@gmail.com>,
	torvalds@linux-foundation.org, kernel-team@android.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ingo Molnar <mingo@kernel.org>, Jann Horn <jann@thejh.net>,
	linux-kselftest@vger.kernel.org, Michal Hocko <mhocko@suse.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Serge Hallyn <serge@hallyn.com>, Shuah Khan <shuah@kernel.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>, Thomas Gleixner <tgl>
Subject: Re: [PATCH v1 1/2] Add polling support to pidfd
Date: Mon, 29 Apr 2019 12:32:59 -0400	[thread overview]
Message-ID: <20190429163259.GA201155@google.com> (raw)
In-Reply-To: <20190429142030.GA17715@redhat.com>

On Mon, Apr 29, 2019 at 04:20:30PM +0200, Oleg Nesterov wrote:
> On 04/29, Joel Fernandes wrote:
> >
> > However, in your code above, it is avoided because we get:
> >
> > Task A (poller)		Task B (exiting task being polled)
> > ------------            ----------------
> > poll() called
> > add_wait_queue()
> > 			exit_state is set to non-zero
> > read exit_state
> > remove_wait_queue()
> > 			wake_up_all()
> 
> just to clarify... No, sys_poll() path doesn't do remove_wait_queue() until
> it returns to user mode, and that is why we can't race with set-exit_code +
> wake_up().

I didn't follow what you mean, the removal from the waitqueue happens in
free_poll_entry() called from poll_freewait() which happens from
do_sys_poll() which is before the syscall returns to user mode. Could you
explain more?

> pidfd_poll() can race with the exiting task, miss exit_code != 0, and return
> zero. However, do_poll() won't block after that and pidfd_poll() will be called
> again.

Here also I didn't follow what you mean. If exit_code is read as 0 in
pidfd_poll(), then in do_poll() the count will be 0 and it will block in
poll_schedule_timeout(). Right? But above you're saying it wont block.
Also if you could show a timing diagram of this different race you're talking
about, that will make things clear. It is a bit hard for me to picture
otherwise.

Also, I will use task_pid() for getting the pid from the task, as you suggest
in the other thread.

thanks,

- Joel

WARNING: multiple messages have this Message-ID (diff)
From: joel at joelfernandes.org (Joel Fernandes)
Subject: [PATCH v1 1/2] Add polling support to pidfd
Date: Mon, 29 Apr 2019 12:32:59 -0400	[thread overview]
Message-ID: <20190429163259.GA201155@google.com> (raw)
In-Reply-To: <20190429142030.GA17715@redhat.com>

On Mon, Apr 29, 2019 at 04:20:30PM +0200, Oleg Nesterov wrote:
> On 04/29, Joel Fernandes wrote:
> >
> > However, in your code above, it is avoided because we get:
> >
> > Task A (poller)		Task B (exiting task being polled)
> > ------------            ----------------
> > poll() called
> > add_wait_queue()
> > 			exit_state is set to non-zero
> > read exit_state
> > remove_wait_queue()
> > 			wake_up_all()
> 
> just to clarify... No, sys_poll() path doesn't do remove_wait_queue() until
> it returns to user mode, and that is why we can't race with set-exit_code +
> wake_up().

I didn't follow what you mean, the removal from the waitqueue happens in
free_poll_entry() called from poll_freewait() which happens from
do_sys_poll() which is before the syscall returns to user mode. Could you
explain more?

> pidfd_poll() can race with the exiting task, miss exit_code != 0, and return
> zero. However, do_poll() won't block after that and pidfd_poll() will be called
> again.

Here also I didn't follow what you mean. If exit_code is read as 0 in
pidfd_poll(), then in do_poll() the count will be 0 and it will block in
poll_schedule_timeout(). Right? But above you're saying it wont block.
Also if you could show a timing diagram of this different race you're talking
about, that will make things clear. It is a bit hard for me to picture
otherwise.

Also, I will use task_pid() for getting the pid from the task, as you suggest
in the other thread.

thanks,

- Joel

WARNING: multiple messages have this Message-ID (diff)
From: joel@joelfernandes.org (Joel Fernandes)
Subject: [PATCH v1 1/2] Add polling support to pidfd
Date: Mon, 29 Apr 2019 12:32:59 -0400	[thread overview]
Message-ID: <20190429163259.GA201155@google.com> (raw)
Message-ID: <20190429163259.Mr-0F8MxZhPAuBCL9KNGWefui4_Dcaqrj6hrT43VT_k@z> (raw)
In-Reply-To: <20190429142030.GA17715@redhat.com>

On Mon, Apr 29, 2019@04:20:30PM +0200, Oleg Nesterov wrote:
> On 04/29, Joel Fernandes wrote:
> >
> > However, in your code above, it is avoided because we get:
> >
> > Task A (poller)		Task B (exiting task being polled)
> > ------------            ----------------
> > poll() called
> > add_wait_queue()
> > 			exit_state is set to non-zero
> > read exit_state
> > remove_wait_queue()
> > 			wake_up_all()
> 
> just to clarify... No, sys_poll() path doesn't do remove_wait_queue() until
> it returns to user mode, and that is why we can't race with set-exit_code +
> wake_up().

I didn't follow what you mean, the removal from the waitqueue happens in
free_poll_entry() called from poll_freewait() which happens from
do_sys_poll() which is before the syscall returns to user mode. Could you
explain more?

> pidfd_poll() can race with the exiting task, miss exit_code != 0, and return
> zero. However, do_poll() won't block after that and pidfd_poll() will be called
> again.

Here also I didn't follow what you mean. If exit_code is read as 0 in
pidfd_poll(), then in do_poll() the count will be 0 and it will block in
poll_schedule_timeout(). Right? But above you're saying it wont block.
Also if you could show a timing diagram of this different race you're talking
about, that will make things clear. It is a bit hard for me to picture
otherwise.

Also, I will use task_pid() for getting the pid from the task, as you suggest
in the other thread.

thanks,

- Joel

WARNING: multiple messages have this Message-ID (diff)
From: Joel Fernandes <joel@joelfernandes.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Christian Brauner <christian@brauner.io>,
	linux-kernel@vger.kernel.org, luto@amacapital.net,
	rostedt@goodmis.org, dancol@google.com, sspatil@google.com,
	jannh@google.com, surenb@google.com, timmurray@google.com,
	Jonathan Kowalski <bl0pbl33p@gmail.com>,
	torvalds@linux-foundation.org, kernel-team@android.com,
	Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ingo Molnar <mingo@kernel.org>, Jann Horn <jann@thejh.net>,
	linux-kselftest@vger.kernel.org, Michal Hocko <mhocko@suse.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Serge Hallyn <serge@hallyn.com>, Shuah Khan <shuah@kernel.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tycho Andersen <tycho@tycho.ws>,
	viro@zeniv.linux.org.uk, linux-api@vger.kernel.org
Subject: Re: [PATCH v1 1/2] Add polling support to pidfd
Date: Mon, 29 Apr 2019 12:32:59 -0400	[thread overview]
Message-ID: <20190429163259.GA201155@google.com> (raw)
In-Reply-To: <20190429142030.GA17715@redhat.com>

On Mon, Apr 29, 2019 at 04:20:30PM +0200, Oleg Nesterov wrote:
> On 04/29, Joel Fernandes wrote:
> >
> > However, in your code above, it is avoided because we get:
> >
> > Task A (poller)		Task B (exiting task being polled)
> > ------------            ----------------
> > poll() called
> > add_wait_queue()
> > 			exit_state is set to non-zero
> > read exit_state
> > remove_wait_queue()
> > 			wake_up_all()
> 
> just to clarify... No, sys_poll() path doesn't do remove_wait_queue() until
> it returns to user mode, and that is why we can't race with set-exit_code +
> wake_up().

I didn't follow what you mean, the removal from the waitqueue happens in
free_poll_entry() called from poll_freewait() which happens from
do_sys_poll() which is before the syscall returns to user mode. Could you
explain more?

> pidfd_poll() can race with the exiting task, miss exit_code != 0, and return
> zero. However, do_poll() won't block after that and pidfd_poll() will be called
> again.

Here also I didn't follow what you mean. If exit_code is read as 0 in
pidfd_poll(), then in do_poll() the count will be 0 and it will block in
poll_schedule_timeout(). Right? But above you're saying it wont block.
Also if you could show a timing diagram of this different race you're talking
about, that will make things clear. It is a bit hard for me to picture
otherwise.

Also, I will use task_pid() for getting the pid from the task, as you suggest
in the other thread.

thanks,

- Joel


  reply	other threads:[~2019-04-29 16:32 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-25 19:00 [PATCH v1 1/2] Add polling support to pidfd joel
2019-04-25 19:00 ` Joel Fernandes (Google)
2019-04-25 19:00 ` Joel Fernandes (Google)
2019-04-25 19:00 ` [PATCH v1 2/2] Add selftests for pidfd polling joel
2019-04-25 19:00   ` Joel Fernandes (Google)
2019-04-25 19:00   ` Joel Fernandes (Google)
2019-04-25 20:00   ` tycho
2019-04-25 20:00     ` Tycho Andersen
2019-04-25 20:00     ` Tycho Andersen
2019-04-26 13:47     ` joel
2019-04-26 13:47       ` Joel Fernandes
2019-04-26 13:47       ` Joel Fernandes
2019-04-25 21:29   ` christian
2019-04-25 21:29     ` Christian Brauner
2019-04-25 21:29     ` Christian Brauner
2019-04-25 22:07     ` dancol
2019-04-25 22:07       ` Daniel Colascione
2019-04-25 22:07       ` Daniel Colascione
2019-04-26 17:26       ` joel
2019-04-26 17:26         ` Joel Fernandes
2019-04-26 17:26         ` Joel Fernandes
2019-04-26 19:35         ` dancol
2019-04-26 19:35           ` Daniel Colascione
2019-04-26 19:35           ` Daniel Colascione
2019-04-26 20:31           ` joel
2019-04-26 20:31             ` Joel Fernandes
2019-04-26 20:31             ` Joel Fernandes
2019-04-26 13:42     ` joel
2019-04-26 13:42       ` Joel Fernandes
2019-04-26 13:42       ` Joel Fernandes
2019-04-25 22:24 ` [PATCH v1 1/2] Add polling support to pidfd Christian Brauner
2019-04-25 22:24   ` Christian Brauner
2019-04-25 22:24   ` Christian Brauner
2019-04-25 22:24   ` christian
2019-04-26 14:23   ` Joel Fernandes
2019-04-26 14:23     ` Joel Fernandes
2019-04-26 14:23     ` Joel Fernandes
2019-04-26 14:23     ` joel
2019-04-26 15:21     ` Christian Brauner
2019-04-26 15:21       ` Christian Brauner
2019-04-26 15:21       ` Christian Brauner
2019-04-26 15:21       ` christian
2019-04-26 15:31       ` Christian Brauner
2019-04-26 15:31         ` Christian Brauner
2019-04-26 15:31         ` Christian Brauner
2019-04-26 15:31         ` christian
2019-04-28 16:24   ` Oleg Nesterov
2019-04-28 16:24     ` Oleg Nesterov
2019-04-28 16:24     ` Oleg Nesterov
2019-04-28 16:24     ` oleg
2019-04-29 14:02     ` Joel Fernandes
2019-04-29 14:02       ` Joel Fernandes
2019-04-29 14:02       ` Joel Fernandes
2019-04-29 14:02       ` joel
2019-04-29 14:07       ` Joel Fernandes
2019-04-29 14:07         ` Joel Fernandes
2019-04-29 14:07         ` Joel Fernandes
2019-04-29 14:07         ` joel
2019-04-29 14:25         ` Oleg Nesterov
2019-04-29 14:25           ` Oleg Nesterov
2019-04-29 14:25           ` Oleg Nesterov
2019-04-29 14:25           ` oleg
2019-04-29 14:20       ` Oleg Nesterov
2019-04-29 14:20         ` Oleg Nesterov
2019-04-29 14:20         ` Oleg Nesterov
2019-04-29 14:20         ` oleg
2019-04-29 16:32         ` Joel Fernandes [this message]
2019-04-29 16:32           ` Joel Fernandes
2019-04-29 16:32           ` Joel Fernandes
2019-04-29 16:32           ` joel
2019-04-30 11:53           ` Oleg Nesterov
2019-04-30 11:53             ` Oleg Nesterov
2019-04-30 11:53             ` Oleg Nesterov
2019-04-30 11:53             ` oleg
2019-04-30 12:07             ` Oleg Nesterov
2019-04-30 12:07               ` Oleg Nesterov
2019-04-30 12:07               ` Oleg Nesterov
2019-04-30 12:07               ` oleg
2019-04-30 15:49             ` Joel Fernandes
2019-04-30 15:49               ` Joel Fernandes
2019-04-30 15:49               ` Joel Fernandes
2019-04-30 15:49               ` joel
2019-04-26 14:58 ` christian
2019-04-26 14:58   ` Christian Brauner
2019-04-26 14:58   ` Christian Brauner

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=20190429163259.GA201155@google.com \
    --to=joel@joelfernandes.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bl0pbl33p@gmail.com \
    --cc=christian@brauner.io \
    --cc=dancol@google.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jann@thejh.net \
    --cc=jannh@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mhocko@suse.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=serge@hallyn.com \
    --cc=sfr@canb.auug.org.au \
    --cc=shuah@kernel.org \
    --cc=sspatil@google.com \
    --cc=surenb@google.com \
    --cc=timmurray@google.com \
    --cc=torvalds@linux-foundation.org \
    /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.