Thread (25 messages) 25 messages, 2 authors, 2015-06-01

Re: [PATCH] mm/oom: Suppress unnecessary "sharing same memory" message.

From: Michal Hocko <hidden>
Date: 2015-06-01 09:03:45

On Sat 30-05-15 02:20:23, Tetsuo Handa wrote:
Michal Hocko wrote:
quoted
On Fri 29-05-15 21:40:47, Tetsuo Handa wrote:
[...]
quoted
quoted
Is it possible that thread1 is doing memory allocation between
down_write(&current->mm->mmap_sem) and up_write(&current->mm->mmap_sem),
thread2 sharing the same mm is waiting at down_read(&current->mm->mmap_sem),
and the OOM killer invoked by thread3 chooses thread2 as the OOM victim and
sets TIF_MEMDIE to thread2?
Your usage of thread is confusing. Threads are of no concerns because
those get killed when the group leader is killed. If you refer to
processes then this is exactly what is handled by:
        for_each_process(p)
                if (p->mm == mm && !same_thread_group(p, victim) &&
                    !(p->flags & PF_KTHREAD)) {
                        if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
                                continue;

                        task_lock(p);   /* Protect ->comm from prctl() */
                        pr_err("Kill process %d (%s) sharing same memory\n",
                                task_pid_nr(p), p->comm);
                        task_unlock(p);
                        do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
                }
I refer to both "Thread-1 in process-1, thread-2 in process-1" case and
"thread-1 in process-1, thread-2 in process-2" case. Thread-3 can be in
process-1 or process-2 or neither.
And that makes it confusing because threads in the same thread group
case is not really interesting. All the threads have fatal signal
pending and they would get access to memory reserves as they hit the oom
killer.

If the mm sharing tasks are separate processes though we have to emulate
that behavior because they do not share the signal handling. That is
what the above for_each_process does. Again they will get access to
memory reserves once they hit the oom killer due to fatal signal
pending.

See? So the only remaining case is when we do not kill other processes
sharing the mm because as you pointed out, the first task even didn't go
through that for_each_process loop.
When TIF_MEMDIE is set to only thread-2 waiting at down_read(), thread-1
between down_write() and up_write() cannot complete memory allocation.
The group leader is not important here
Yes you are right. I thought that sending SIGKILL to !leader will not
kill the whole group but now that I am looking at complete_signal it
really does (SIGKILL is always a group signal). So group leader really
doesn't play any role here. Sorry about the confusion.
because I'm talking about situations
when individual thread cannot arrive at exit_mm() after receiving SIGKILL
due to lock dependency.
quoted
But this is a real corner case. It would have to be current to trigger
OOM killer and the userspace would have to be able to send the signal
at the right moment... So I am even not sure this needs fixing. Are you
able to trigger it?
I'm not sure whether we are talking about the same problem.
I thought that we could get rid of TIF_MEMDIE like

    for_each_process(p) {
            if (p->mm == thread2->mm && !(p->flags & PF_KTHREAD) &&
                p->signal->oom_score_adj != OOM_SCORE_ADJ_MIN)
                        do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
    }
    thread2->mm->chosen_by_oom_killer = true;
why would you send SIGKILL to your threads in the same thread group again?
if we need to set TIF_MEMDIE to all threads like

    for_each_process(p) {
            if (p->mm == thread2->mm && !(p->flags & PF_KTHREAD) &&
                p->signal->oom_score_adj != OOM_SCORE_ADJ_MIN) {
                        do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
                        for_each_thread(p, t)
                                mark_oom_victim(t);
            }
    }

in order to make sure that thread-1 can complete memory allocation.
No we can't. See the big fat warning above the for_each_process loop.
 
-- 
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help