Re: [PATCH rc5-mm] pids: kill PIDTYPE_TGID

[ebiederm@xmission.com (Eric W. Biederman)]

Oleg Nesterov <oleg@tv-sign.ru> writes:

> Eric W. Biederman wrote:
>>
>> >               p->group_leader = current->group_leader;
>> > +             list_add_tail_rcu(&p->thread_group, &current->thread_group);
>> Can this be:
>> list_add_tail_rcu(&p->thread_group, &current->group_leader->thread_group);
>
> Done.
>
>> That way at least the odds of missing a new task_struct when doing an
>> rcu traversal are reduced almost to 0.
>
> Am I understand correctly? This change has effect when we are doing the
> traversal starting from ->group_leader in a "best effort" manner lockless,
> yes?

Yes.  The important point for reasoning about it is that we have a fixed
point that we append or prepend to.

I think I have finally figured out the invariants we will need to
send signals using just the rcu_read_lock().  In that case we want to
traverse the list in such a way that we are guaranteed to never see
new entries.  That gives us an atomic snapshot of the tasks to send
signals to, and it gives us a progress guarantee.

Now I need to look and see if any one has documented any rules
regarding atomicity of sending signals to a group of processes.


> [PATCH rc5-mm] pids: kill PIDTYPE_TGID
>
> depends on pidhash-dont-count-idle-threads.patch
>
> This patch kills PIDTYPE_TGID pid_type thus saving one hash table
> in kernel/pid.c and speeding up subthreads create/destroy a bit.
> It is also a preparation for the further tref/pids rework.
>
> This patch adds 'struct list_head thread_group' to 'struct task_struct'
> instead.
>
> We don't detach group leader from PIDTYPE_PID namespace until another
> thread inherits it's ->pid == ->tgid, so we are safe wrt premature
> free_pidmap(->tgid) call.
>
> Currently there are no users of find_task_by_pid_type(PIDTYPE_TGID).
> Should the need arise, we can use find_task_by_pid()->group_leader.
>
>  include/linux/pid.h   |    1 -
>  include/linux/sched.h |   11 ++++++++---
>  kernel/exit.c         |   10 +---------
>  kernel/fork.c         |    4 +++-
>  4 files changed, 12 insertions(+), 14 deletions(-)
>
> Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

Acked-By: Eric Biederman <ebiederm@xmission.com>