On 06/24, Tetsuo Handa wrote: > > On CONFIG_MMU=n kernels, nothing > will clear TIF_MEMDIE and the system can OOM livelock if TIF_MEMDIE was > by error set to a mm-less thread group leader. and btw this needs more cleanups imo. I mean, the fact we pass task_struct to wake_oom_reaper() looks, well, strange. But this is off-topic right now. > @@ -839,9 +839,19 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p, > * its children or threads, just set TIF_MEMDIE so it can die quickly > */ > if (task_will_free_mem(p)) { > - mark_oom_victim(p); > - wake_oom_reaper(p); > - put_task_struct(p); > + p = find_lock_task_mm(victim); > + if (!p) { Well, this doesn't really matter... but imo victim = find_lock_task_mm(p); if (!victim) { will look much more readable, and this way we won't depend on the early "victim = p" initialization at the start. > + put_task_struct(victim); > + return; > + } else if (victim != p) { > + get_task_struct(p); > + put_task_struct(victim); > + victim = p; > + } Tetsuo but this is horrible ;) At least this needs a comment to explain _why_. Because this looks "obviously unnecessary"; exit_oom_victim() does find_lock_task_mm() too and "p" can exit right after we drop task_lock(). Not to mention that task_will_free_mem() called find_lock_task_mm() right before that, so this is sub-optimal in any case. IOW, this should explain that we only need this for mark_oom_victim(), and only if CONFIG_MMU=n. And this leads to other questions: - Why we can livelock in this case? This should be documented too imo, I have to admit I don't understand why. But yes, yes, sorry, I ignored a lot of emails in this area :/ - Why mark_oom_victim() can't check ->mm != NULL itself? Oleg. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>