Messages in this thread |  | | Date | Tue, 4 Oct 2016 18:21:14 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/4] mm, oom: do not rely on TIF_MEMDIE for exit_oom_victim |
| |
On 10/04, Michal Hocko wrote: > > -void release_task(struct task_struct *p) > +bool release_task(struct task_struct *p) > { > struct task_struct *leader; > int zap_leader; > + bool last = false; > repeat: > /* don't need to get the RCU readlock here - the process is dead and > * can't be modifying its own credentials. But shut RCU-lockdep up */ > @@ -197,8 +198,10 @@ void release_task(struct task_struct *p) > * then we are the one who should release the leader. > */ > zap_leader = do_notify_parent(leader, leader->exit_signal); > - if (zap_leader) > + if (zap_leader) { > leader->exit_state = EXIT_DEAD; > + last = true; > + } > }
This looks strange... it won't return true if "p" is the group leader.
> @@ -584,12 +587,15 @@ static void forget_original_parent(struct task_struct *father, > /* > * Send signals to all our closest relatives so that they know > * to properly mourn us.. > + * > + * Returns true if this is the last thread from the thread group > */ > -static void exit_notify(struct task_struct *tsk, int group_dead) > +static bool exit_notify(struct task_struct *tsk, int group_dead) > { > bool autoreap; > struct task_struct *p, *n; > LIST_HEAD(dead); > + bool last = false; > > write_lock_irq(&tasklist_lock); > forget_original_parent(tsk, &dead); > @@ -606,6 +612,7 @@ static void exit_notify(struct task_struct *tsk, int group_dead) > } else if (thread_group_leader(tsk)) { > autoreap = thread_group_empty(tsk) && > do_notify_parent(tsk, tsk->exit_signal); > + last = thread_group_empty(tsk);
so this can't detect the multi-threaded group exit, and ...
> list_for_each_entry_safe(p, n, &dead, ptrace_entry) { > list_del_init(&p->ptrace_entry); > - release_task(p); > + if (release_task(p) && p == tsk) > + last = true;
this can only happen if this process auto-reaps itself. Not to mention that exit_notify() will never return true if traced.
No, this doesn't look right.
Oleg.
|  |