All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Kowalski <bl0pbl33p@gmail.com>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: Oleg Nesterov <oleg@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	rostedt@goodmis.org, Daniel Colascione <dancol@google.com>,
	Christian Brauner <christian@brauner.io>,
	Jann Horn <jannh@google.com>,
	surenb@google.com, Linus Torvalds <torvalds@linux-foundation.org>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrei Vagin <avagin@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	Michal Hocko <mhocko@suse.com>, Nadav Amit <namit@vmware.com>,
	Serge Hallyn <serge@hallyn.com>, Shuah Khan <shuah@kernel.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Taehee Yoo <ap420073@gmail.com>, Tejun Heo <tj@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	kernel-team@android.com, Tycho Andersen <tycho@tycho.ws>
Subject: Re: [PATCH RFC 1/2] Add polling support to pidfd
Date: Thu, 18 Apr 2019 19:44:33 +0100	[thread overview]
Message-ID: <CAGLj2rELsr0PUgu41uZX_TKCpT-Q00X+cvgBhJ-U7RU3EBo5tA@mail.gmail.com> (raw)
In-Reply-To: <20190416192051.GA184889@google.com>

On Tue, Apr 16, 2019 at 8:21 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Tue, Apr 16, 2019 at 02:04:31PM +0200, Oleg Nesterov wrote:
> > On 04/11, Joel Fernandes (Google) wrote:
> > >
> > > +static unsigned int proc_tgid_base_poll(struct file *file, struct poll_table_struct *pts)
> > > +{
> > > +   int poll_flags = 0;
> > > +   struct task_struct *task;
> > > +   struct pid *pid;
> > > +
> > > +   task = get_proc_task(file->f_path.dentry->d_inode);
> > > +
> > > +   WARN_ON_ONCE(task && !thread_group_leader(task));
> > > +
> > > +   /*
> > > +    * tasklist_lock must be held because to avoid racing with
> > > +    * changes in exit_state and wake up. Basically to avoid:
> > > +    *
> > > +    * P0: read exit_state = 0
> > > +    * P1: write exit_state = EXIT_DEAD
> > > +    * P1: Do a wake up - wq is empty, so do nothing
> > > +    * P0: Queue for polling - wait forever.
> > > +    */
> > > +   read_lock(&tasklist_lock);
> > > +   if (!task)
> > > +           poll_flags = POLLIN | POLLRDNORM | POLLERR;
> > > +   else if (task->exit_state == EXIT_DEAD)
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +   else if (task->exit_state == EXIT_ZOMBIE && thread_group_empty(task))
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +
> > > +   if (!poll_flags) {
> > > +           pid = proc_pid(file->f_path.dentry->d_inode);
> > > +           poll_wait(file, &pid->wait_pidfd, pts);
> > > +   }
> >
> > can't understand...
> >
> > Could you explain when it should return POLLIN? When the whole process exits?
>
> It returns POLLIN when the task is dead or doesn't exist anymore, or when it
> is in a zombie state and there's no other thread in the thread group.
>

Would using something other than POLLIN be an option (maybe POLLPRI)?
The convention is to use it to indicate readability on the descriptor,
and also possibly POLLHUP instead of POLLERR (the latter is less of a
problem, but FreeBSD also does the same, so it'd help with some
consistency for libraries wanting to use this, which aren't interested
in other sub states).

>
>  - Joel
>

WARNING: multiple messages have this Message-ID (diff)
From: bl0pbl33p at gmail.com (Jonathan Kowalski)
Subject: [PATCH RFC 1/2] Add polling support to pidfd
Date: Thu, 18 Apr 2019 19:44:33 +0100	[thread overview]
Message-ID: <CAGLj2rELsr0PUgu41uZX_TKCpT-Q00X+cvgBhJ-U7RU3EBo5tA@mail.gmail.com> (raw)
In-Reply-To: <20190416192051.GA184889@google.com>

On Tue, Apr 16, 2019 at 8:21 PM Joel Fernandes <joel at joelfernandes.org> wrote:
>
> On Tue, Apr 16, 2019 at 02:04:31PM +0200, Oleg Nesterov wrote:
> > On 04/11, Joel Fernandes (Google) wrote:
> > >
> > > +static unsigned int proc_tgid_base_poll(struct file *file, struct poll_table_struct *pts)
> > > +{
> > > +   int poll_flags = 0;
> > > +   struct task_struct *task;
> > > +   struct pid *pid;
> > > +
> > > +   task = get_proc_task(file->f_path.dentry->d_inode);
> > > +
> > > +   WARN_ON_ONCE(task && !thread_group_leader(task));
> > > +
> > > +   /*
> > > +    * tasklist_lock must be held because to avoid racing with
> > > +    * changes in exit_state and wake up. Basically to avoid:
> > > +    *
> > > +    * P0: read exit_state = 0
> > > +    * P1: write exit_state = EXIT_DEAD
> > > +    * P1: Do a wake up - wq is empty, so do nothing
> > > +    * P0: Queue for polling - wait forever.
> > > +    */
> > > +   read_lock(&tasklist_lock);
> > > +   if (!task)
> > > +           poll_flags = POLLIN | POLLRDNORM | POLLERR;
> > > +   else if (task->exit_state == EXIT_DEAD)
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +   else if (task->exit_state == EXIT_ZOMBIE && thread_group_empty(task))
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +
> > > +   if (!poll_flags) {
> > > +           pid = proc_pid(file->f_path.dentry->d_inode);
> > > +           poll_wait(file, &pid->wait_pidfd, pts);
> > > +   }
> >
> > can't understand...
> >
> > Could you explain when it should return POLLIN? When the whole process exits?
>
> It returns POLLIN when the task is dead or doesn't exist anymore, or when it
> is in a zombie state and there's no other thread in the thread group.
>

Would using something other than POLLIN be an option (maybe POLLPRI)?
The convention is to use it to indicate readability on the descriptor,
and also possibly POLLHUP instead of POLLERR (the latter is less of a
problem, but FreeBSD also does the same, so it'd help with some
consistency for libraries wanting to use this, which aren't interested
in other sub states).

>
>  - Joel
>

WARNING: multiple messages have this Message-ID (diff)
From: bl0pbl33p@gmail.com (Jonathan Kowalski)
Subject: [PATCH RFC 1/2] Add polling support to pidfd
Date: Thu, 18 Apr 2019 19:44:33 +0100	[thread overview]
Message-ID: <CAGLj2rELsr0PUgu41uZX_TKCpT-Q00X+cvgBhJ-U7RU3EBo5tA@mail.gmail.com> (raw)
Message-ID: <20190418184433.vbMAydqYec2wUYGjaYSPhSwBJuhwegrMIPLRwfZMNkE@z> (raw)
In-Reply-To: <20190416192051.GA184889@google.com>

On Tue, Apr 16, 2019@8:21 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Tue, Apr 16, 2019@02:04:31PM +0200, Oleg Nesterov wrote:
> > On 04/11, Joel Fernandes (Google) wrote:
> > >
> > > +static unsigned int proc_tgid_base_poll(struct file *file, struct poll_table_struct *pts)
> > > +{
> > > +   int poll_flags = 0;
> > > +   struct task_struct *task;
> > > +   struct pid *pid;
> > > +
> > > +   task = get_proc_task(file->f_path.dentry->d_inode);
> > > +
> > > +   WARN_ON_ONCE(task && !thread_group_leader(task));
> > > +
> > > +   /*
> > > +    * tasklist_lock must be held because to avoid racing with
> > > +    * changes in exit_state and wake up. Basically to avoid:
> > > +    *
> > > +    * P0: read exit_state = 0
> > > +    * P1: write exit_state = EXIT_DEAD
> > > +    * P1: Do a wake up - wq is empty, so do nothing
> > > +    * P0: Queue for polling - wait forever.
> > > +    */
> > > +   read_lock(&tasklist_lock);
> > > +   if (!task)
> > > +           poll_flags = POLLIN | POLLRDNORM | POLLERR;
> > > +   else if (task->exit_state == EXIT_DEAD)
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +   else if (task->exit_state == EXIT_ZOMBIE && thread_group_empty(task))
> > > +           poll_flags = POLLIN | POLLRDNORM;
> > > +
> > > +   if (!poll_flags) {
> > > +           pid = proc_pid(file->f_path.dentry->d_inode);
> > > +           poll_wait(file, &pid->wait_pidfd, pts);
> > > +   }
> >
> > can't understand...
> >
> > Could you explain when it should return POLLIN? When the whole process exits?
>
> It returns POLLIN when the task is dead or doesn't exist anymore, or when it
> is in a zombie state and there's no other thread in the thread group.
>

Would using something other than POLLIN be an option (maybe POLLPRI)?
The convention is to use it to indicate readability on the descriptor,
and also possibly POLLHUP instead of POLLERR (the latter is less of a
problem, but FreeBSD also does the same, so it'd help with some
consistency for libraries wanting to use this, which aren't interested
in other sub states).

>
>  - Joel
>

  parent reply	other threads:[~2019-04-18 18:44 UTC|newest]

Thread overview: 198+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 17:50 [PATCH RFC 1/2] Add polling support to pidfd Joel Fernandes (Google)
2019-04-11 17:50 ` Joel Fernandes (Google)
2019-04-11 17:50 ` joel
2019-04-11 17:50 ` [PATCH RFC 2/2] Add selftests for pidfd polling Joel Fernandes (Google)
2019-04-11 17:50   ` Joel Fernandes (Google)
2019-04-11 17:50   ` joel
2019-04-12 14:51   ` Tycho Andersen
2019-04-12 14:51     ` Tycho Andersen
2019-04-12 14:51     ` tycho
2019-04-11 20:00 ` [PATCH RFC 1/2] Add polling support to pidfd Joel Fernandes
2019-04-11 20:00   ` Joel Fernandes
2019-04-11 20:00   ` joel
2019-04-11 20:02   ` Christian Brauner
2019-04-11 20:02     ` Christian Brauner
2019-04-11 20:02     ` christian
2019-04-11 20:20     ` Joel Fernandes
2019-04-11 20:20       ` Joel Fernandes
2019-04-11 20:20       ` joel
2019-04-12 21:32 ` Andy Lutomirski
2019-04-12 21:32   ` Andy Lutomirski
2019-04-12 21:32   ` luto
2019-04-13  0:09   ` Joel Fernandes
2019-04-13  0:09     ` Joel Fernandes
2019-04-13  0:09     ` joel
     [not found]     ` <CAKOZuetX4jMPDtDqAvGgSNo4BHf9BOnu79ufEiULfM5X5nDyyQ@mail.gmail.com>
2019-04-13  0:56       ` Daniel Colascione
2019-04-13  0:56         ` Daniel Colascione
2019-04-13  0:56         ` dancol
2019-04-14 18:19   ` Linus Torvalds
2019-04-14 18:19     ` Linus Torvalds
2019-04-14 18:19     ` torvalds
2019-04-16 12:04 ` Oleg Nesterov
2019-04-16 12:04   ` Oleg Nesterov
2019-04-16 12:04   ` oleg
2019-04-16 12:43   ` Oleg Nesterov
2019-04-16 12:43     ` Oleg Nesterov
2019-04-16 12:43     ` oleg
2019-04-16 19:20   ` Joel Fernandes
2019-04-16 19:20     ` Joel Fernandes
2019-04-16 19:20     ` joel
2019-04-16 19:32     ` Joel Fernandes
2019-04-16 19:32       ` Joel Fernandes
2019-04-16 19:32       ` joel
2019-04-17 13:09     ` Oleg Nesterov
2019-04-17 13:09       ` Oleg Nesterov
2019-04-17 13:09       ` oleg
2019-04-18 17:23       ` Jann Horn
2019-04-18 17:23         ` Jann Horn
2019-04-18 17:23         ` jannh
2019-04-18 17:26         ` Christian Brauner
2019-04-18 17:26           ` Christian Brauner
2019-04-18 17:26           ` christian
2019-04-18 17:53           ` Daniel Colascione
2019-04-18 17:53             ` Daniel Colascione
2019-04-18 17:53             ` dancol
2019-04-19 19:02           ` Joel Fernandes
2019-04-19 19:02             ` Joel Fernandes
2019-04-19 19:02             ` joel
2019-04-19 19:18             ` Christian Brauner
2019-04-19 19:18               ` Christian Brauner
2019-04-19 19:18               ` christian
2019-04-19 19:22               ` Christian Brauner
2019-04-19 19:22                 ` Christian Brauner
2019-04-19 19:22                 ` christian
2019-04-19 19:42                 ` Christian Brauner
2019-04-19 19:42                   ` Christian Brauner
2019-04-19 19:42                   ` christian
2019-04-19 19:49               ` Joel Fernandes
2019-04-19 19:49                 ` Joel Fernandes
2019-04-19 19:49                 ` joel
2019-04-19 20:01                 ` Christian Brauner
2019-04-19 20:01                   ` Christian Brauner
2019-04-19 20:01                   ` christian
2019-04-19 21:13                   ` Joel Fernandes
2019-04-19 21:13                     ` Joel Fernandes
2019-04-19 21:13                     ` joel
2019-04-19 20:34                 ` Daniel Colascione
2019-04-19 20:34                   ` Daniel Colascione
2019-04-19 20:34                   ` dancol
2019-04-19 20:57                   ` Christian Brauner
2019-04-19 20:57                     ` Christian Brauner
2019-04-19 20:57                     ` christian
2019-04-19 21:20                     ` Joel Fernandes
2019-04-19 21:20                       ` Joel Fernandes
2019-04-19 21:20                       ` joel
2019-04-19 21:24                       ` Daniel Colascione
2019-04-19 21:24                         ` Daniel Colascione
2019-04-19 21:24                         ` dancol
2019-04-19 21:45                         ` Joel Fernandes
2019-04-19 21:45                           ` Joel Fernandes
2019-04-19 21:45                           ` joel
2019-04-19 22:08                           ` Daniel Colascione
2019-04-19 22:08                             ` Daniel Colascione
2019-04-19 22:08                             ` dancol
2019-04-19 22:17                             ` Christian Brauner
2019-04-19 22:17                               ` Christian Brauner
2019-04-19 22:17                               ` christian
2019-04-19 22:37                               ` Daniel Colascione
2019-04-19 22:37                                 ` Daniel Colascione
2019-04-19 22:37                                 ` dancol
2019-04-24  8:04                         ` Enrico Weigelt, metux IT consult
2019-04-24  8:04                           ` Enrico Weigelt, metux IT consult
2019-04-24  8:04                           ` lkml
2019-04-19 21:59                       ` Christian Brauner
2019-04-19 21:59                         ` Christian Brauner
2019-04-19 21:59                         ` christian
2019-04-20 11:51                         ` Oleg Nesterov
2019-04-20 11:51                           ` Oleg Nesterov
2019-04-20 11:51                           ` oleg
2019-04-20 12:26                           ` Oleg Nesterov
2019-04-20 12:26                             ` Oleg Nesterov
2019-04-20 12:26                             ` oleg
2019-04-20 12:35                             ` Christian Brauner
2019-04-20 12:35                               ` Christian Brauner
2019-04-20 12:35                               ` christian
2019-04-19 23:11                       ` Linus Torvalds
2019-04-19 23:11                         ` Linus Torvalds
2019-04-19 23:11                         ` torvalds
2019-04-19 23:20                         ` Christian Brauner
2019-04-19 23:20                           ` Christian Brauner
2019-04-19 23:20                           ` christian
2019-04-19 23:32                           ` Linus Torvalds
2019-04-19 23:32                             ` Linus Torvalds
2019-04-19 23:32                             ` torvalds
2019-04-19 23:36                             ` Daniel Colascione
2019-04-19 23:36                               ` Daniel Colascione
2019-04-19 23:36                               ` dancol
2019-04-20  0:46                         ` Joel Fernandes
2019-04-20  0:46                           ` Joel Fernandes
2019-04-20  0:46                           ` joel
2019-04-19 21:21                     ` Daniel Colascione
2019-04-19 21:21                       ` Daniel Colascione
2019-04-19 21:21                       ` dancol
2019-04-19 21:48                       ` Christian Brauner
2019-04-19 21:48                         ` Christian Brauner
2019-04-19 21:48                         ` christian
2019-04-19 22:02                         ` Christian Brauner
2019-04-19 22:02                           ` Christian Brauner
2019-04-19 22:02                           ` christian
2019-04-19 22:46                           ` Daniel Colascione
2019-04-19 22:46                             ` Daniel Colascione
2019-04-19 22:46                             ` dancol
2019-04-19 23:12                             ` Christian Brauner
2019-04-19 23:12                               ` Christian Brauner
2019-04-19 23:12                               ` christian
2019-04-19 23:46                               ` Daniel Colascione
2019-04-19 23:46                                 ` Daniel Colascione
2019-04-19 23:46                                 ` dancol
2019-04-20  0:17                                 ` Christian Brauner
2019-04-20  0:17                                   ` Christian Brauner
2019-04-20  0:17                                   ` christian
2019-04-24  9:05                                   ` Enrico Weigelt, metux IT consult
2019-04-24  9:05                                     ` Enrico Weigelt, metux IT consult
2019-04-24  9:05                                     ` lkml
2019-04-24  9:03                                 ` Enrico Weigelt, metux IT consult
2019-04-24  9:03                                   ` Enrico Weigelt, metux IT consult
2019-04-24  9:03                                   ` lkml
2019-04-19 22:35                         ` Daniel Colascione
2019-04-19 22:35                           ` Daniel Colascione
2019-04-19 22:35                           ` dancol
2019-04-19 23:02                           ` Christian Brauner
2019-04-19 23:02                             ` Christian Brauner
2019-04-19 23:02                             ` christian
2019-04-19 23:29                             ` Daniel Colascione
2019-04-19 23:29                               ` Daniel Colascione
2019-04-19 23:29                               ` dancol
2019-04-20  0:02                               ` Christian Brauner
2019-04-20  0:02                                 ` Christian Brauner
2019-04-20  0:02                                 ` christian
2019-04-24  9:17                               ` Enrico Weigelt, metux IT consult
2019-04-24  9:17                                 ` Enrico Weigelt, metux IT consult
2019-04-24  9:17                                 ` lkml
2019-04-24  9:11                             ` Enrico Weigelt, metux IT consult
2019-04-24  9:11                               ` Enrico Weigelt, metux IT consult
2019-04-24  9:11                               ` lkml
2019-04-24  8:56                         ` Enrico Weigelt, metux IT consult
2019-04-24  8:56                           ` Enrico Weigelt, metux IT consult
2019-04-24  8:56                           ` lkml
2019-04-24  8:20                       ` Enrico Weigelt, metux IT consult
2019-04-24  8:20                         ` Enrico Weigelt, metux IT consult
2019-04-24  8:20                         ` lkml
2019-04-19 15:43         ` Oleg Nesterov
2019-04-19 15:43           ` Oleg Nesterov
2019-04-19 15:43           ` oleg
2019-04-19 18:12       ` Joel Fernandes
2019-04-19 18:12         ` Joel Fernandes
2019-04-19 18:12         ` joel
2019-04-18 18:44     ` Jonathan Kowalski [this message]
2019-04-18 18:44       ` Jonathan Kowalski
2019-04-18 18:44       ` bl0pbl33p
2019-04-18 18:57       ` Daniel Colascione
2019-04-18 18:57         ` Daniel Colascione
2019-04-18 18:57         ` dancol
2019-04-18 19:14         ` Linus Torvalds
2019-04-18 19:14           ` Linus Torvalds
2019-04-18 19:14           ` torvalds
2019-04-19 19:05           ` Joel Fernandes
2019-04-19 19:05             ` Joel Fernandes
2019-04-19 19:05             ` joel

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=CAGLj2rELsr0PUgu41uZX_TKCpT-Q00X+cvgBhJ-U7RU3EBo5tA@mail.gmail.com \
    --to=bl0pbl33p@gmail.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=ap420073@gmail.com \
    --cc=arnd@arndb.de \
    --cc=avagin@gmail.com \
    --cc=christian@brauner.io \
    --cc=dancol@google.com \
    --cc=ebiederm@xmission.com \
    --cc=jannh@google.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mhocko@suse.com \
    --cc=namit@vmware.com \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=serge@hallyn.com \
    --cc=sfr@canb.auug.org.au \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tycho@tycho.ws \
    --cc=viro@zeniv.linux.org.uk \
    /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.