Messages in this thread Patch in this message | | | Date | Thu, 1 Oct 2015 15:24:58 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH -mm v2 3/3] mm/oom_kill: fix the wrong task->mm == mm checks in oom_kill_process() |
| |
On Wed, 30 Sep 2015 20:24:11 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
> Both "child->mm == mm" and "p->mm != mm" checks in oom_kill_process() > are wrong. task->mm can be NULL if the task is the exited group leader. > This means in particular that "kill sharing same memory" loop can miss > a process with a zombie leader which uses the same ->mm. > > Note: the process_has_mm(child, p->mm) check is still not 100% correct, > p->mm can be NULL too. This is minor, but probably deserves a fix or a > comment anyway. > > ... > > +static bool process_shares_mm(struct task_struct *p, struct mm_struct *mm) > +{ > + struct task_struct *t; > + > + for_each_thread(p, t) { > + struct mm_struct *t_mm = READ_ONCE(t->mm); > + if (t_mm) > + return t_mm == mm; > + } > + return false; > +}
Guys, please don't write write-only code. This function is deeply unobvious and I don't think a typical reader will have a hope of understanding why things are this way.
I had a lame attempt:
--- a/mm/oom_kill.c~mm-oom_kill-fix-the-wrong-task-mm-==-mm-checks-in-oom_kill_process-fix +++ a/mm/oom_kill.c @@ -483,6 +483,12 @@ void oom_killer_enable(void) oom_killer_disabled = false; } +/* + * task->mm can be NULL if the task is the exited group leader. So to + * determine whether the task is using a particular mm, we examine all the + * task's threads: if one of those is using this mm then this task was also + * using it. + */ static bool process_shares_mm(struct task_struct *p, struct mm_struct *mm) { struct task_struct *t; _ Which makes me wonder if "process_shared_mm" or even "process_used_to_share_mm" would be better names...
| |