Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Oleg Nesterov
On 05/31, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > OK. But this means that even 1/3 is not 100% right, exactly because > > leader can be unhashed right before first_tid() takes rcu lock. Easy > > to fix, we should simply factor out the "nr != 0" check. > > > > And this also means

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Eric W. Biederman
Oleg Nesterov writes: > Eric, sorry for delay. > > On 05/29, Eric W. Biederman wrote: >> >> Oleg Nesterov writes: >> >> > Why the empty "." + ".." dir is bad if the task(s) has gone away after >> > opendir? >> >> Because the definition of a deleted directory that you are in is that >> getdents

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Oleg Nesterov
Eric, sorry for delay. On 05/29, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > Why the empty "." + ".." dir is bad if the task(s) has gone away after > > opendir? > > Because the definition of a deleted directory that you are in is that > getdents will return -ENOENT. > > You can

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Oleg Nesterov
Eric, sorry for delay. On 05/29, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: Why the empty . + .. dir is bad if the task(s) has gone away after opendir? Because the definition of a deleted directory that you are in is that getdents will return -ENOENT. You can

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Eric W. Biederman
Oleg Nesterov o...@redhat.com writes: Eric, sorry for delay. On 05/29, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: Why the empty . + .. dir is bad if the task(s) has gone away after opendir? Because the definition of a deleted directory that you are in is that

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-31 Thread Oleg Nesterov
On 05/31, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: OK. But this means that even 1/3 is not 100% right, exactly because leader can be unhashed right before first_tid() takes rcu lock. Easy to fix, we should simply factor out the nr != 0 check. And this also means

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-29 Thread Eric W. Biederman
Oleg Nesterov writes: > On 05/28, Eric W. Biederman wrote: >> >> Oleg Nesterov writes: >> >> > proc_task_readdir() does not really need "leader", first_tid() >> > has to revalidate it anyway. Just pass proc_pid(inode) to >> > first_tid() instead, it can do pid_task(PIDTYPE_PID) itself >> > and

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-29 Thread Oleg Nesterov
On 05/28, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > > proc_task_readdir() does not really need "leader", first_tid() > > has to revalidate it anyway. Just pass proc_pid(inode) to > > first_tid() instead, it can do pid_task(PIDTYPE_PID) itself > > and read ->group_leader only if

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-29 Thread Oleg Nesterov
On 05/28, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: proc_task_readdir() does not really need leader, first_tid() has to revalidate it anyway. Just pass proc_pid(inode) to first_tid() instead, it can do pid_task(PIDTYPE_PID) itself and read -group_leader only if

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-29 Thread Eric W. Biederman
Oleg Nesterov o...@redhat.com writes: On 05/28, Eric W. Biederman wrote: Oleg Nesterov o...@redhat.com writes: proc_task_readdir() does not really need leader, first_tid() has to revalidate it anyway. Just pass proc_pid(inode) to first_tid() instead, it can do pid_task(PIDTYPE_PID)

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-28 Thread Eric W. Biederman
Oleg Nesterov writes: > proc_task_readdir() does not really need "leader", first_tid() > has to revalidate it anyway. Just pass proc_pid(inode) to > first_tid() instead, it can do pid_task(PIDTYPE_PID) itself > and read ->group_leader only if necessary. > > Note: I am not sure

Re: [PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-28 Thread Eric W. Biederman
Oleg Nesterov o...@redhat.com writes: proc_task_readdir() does not really need leader, first_tid() has to revalidate it anyway. Just pass proc_pid(inode) to first_tid() instead, it can do pid_task(PIDTYPE_PID) itself and read -group_leader only if necessary. Note: I am not sure

[PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-27 Thread Oleg Nesterov
proc_task_readdir() does not really need "leader", first_tid() has to revalidate it anyway. Just pass proc_pid(inode) to first_tid() instead, it can do pid_task(PIDTYPE_PID) itself and read ->group_leader only if necessary. Note: I am not sure proc_task_readdir() really needs the initial -ENOENT

[PATCH 3/3] proc: simplify proc_task_readdir/first_tid paths

2013-05-27 Thread Oleg Nesterov
proc_task_readdir() does not really need leader, first_tid() has to revalidate it anyway. Just pass proc_pid(inode) to first_tid() instead, it can do pid_task(PIDTYPE_PID) itself and read -group_leader only if necessary. Note: I am not sure proc_task_readdir() really needs the initial -ENOENT