Re: [RFC PATCH] oom: Don't count on mm-less current process.

[Michal Hocko <mhocko@suse.cz>]

On Tue 23-12-14 20:46:07, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > > Also, why not to call set_tsk_thread_flag() and do_send_sig_info() together
> > > like below
> > 
> > What would be an advantage? I am not really sure whether the two locks
> > might nest as well.
> 
> I imagined that current thread sets TIF_MEMDIE on a victim thread, then
> sleeps for 30 seconds immediately after task_unlock() (it's an overdone
> delay),

Only if the current task was preempted for such a long time. Which
doesn't sound too probable to me.

> and finally sets SIGKILL on that victim thread. If such a delay
> happened, that victim thread is free to abuse TIF_MEMDIE for that period.
> Thus, I thought sending SIGKILL followed by setting TIF_MEMDIE is better.

I don't know, I can hardly find a scenario where it would make any
difference in the real life. If the victim needs to allocate a memory to
finish then it would trigger OOM again and have to wait/loop until this
OOM killer releases the oom zonelist lock just to find out it already
has TIF_MEMDIE set and can dive into memory reserves. Which way is more
correct is a question but I wouldn't change it without having a really
good reason. This whole code is subtle already, let's not make it even
more so.

> 
>  	rcu_read_unlock();
>  
> -	set_tsk_thread_flag(victim, TIF_MEMDIE);
>  	do_send_sig_info(SIGKILL, SEND_SIG_FORCED, victim, true);
> +	task_lock(victim);
> +	if (victim->mm)
> +		set_tsk_thread_flag(victim, TIF_MEMDIE);
> +	task_unlock(victim);
>  	put_task_struct(victim);
> 
> If such a delay is theoretically impossible, I'm OK with your patch.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>