On Fri 27-05-16 22:18:42, Tetsuo Handa wrote: > Michal Hocko wrote: > > On Fri 27-05-16 19:31:19, Tetsuo Handa wrote: > > > Michal Hocko wrote: > > > > Hi, > > > > I believe that after [1] and this patch we can reasonably expect that > > > > the risk of the oom lockups is so low that we do not need to employ > > > > timeout based solutions. I am sending this as an RFC because there still > > > > might be better ways to accomplish the similar effect. I just like this > > > > one because it is nicely grafted into the oom reaper which will now be > > > > invoked for basically all oom victims. > > > > > > > > [1] http://lkml.kernel.org/r/1464266415-15558-1-git-send-email-mhocko@xxxxxxxxxx > > > > > > I still cannot agree with "we do not need to employ timeout based solutions". > > > > > > While it is true that OOM-reap is per "struct mm_struct" action, we don't > > > need to change user visible oom_score_adj interface by [1] in order to > > > enforce OOM-kill being per "struct mm_struct" action. > > > > We want to change the oom_score_adj behavior for the pure consistency I > > believe. > > Is it an agreed conclusion rather than your will? Did userspace developers ack? If you think this is not the right approach then please comment as a reply to the patch. [...] > > So this is the biggest change to my approach. And I think it is > > incorrect because you cannot simply reap the memory when you have active > > users of that memory potentially. > > I don't reap the memory when I have active users of that memory potentially. > I do below check. I'm calling wake_oom_reaper() in order to guarantee that > oom_reap_task() shall clear TIF_MEMDIE and drop oom_victims. > > @@ -483,7 +527,7 @@ static bool __oom_reap_task(struct task_struct *tsk) > > task_unlock(p); > > - if (!down_read_trylock(&mm->mmap_sem)) { > + if (!mm_is_reapable(mm) || !down_read_trylock(&mm->mmap_sem)) { > ret = false; > goto unlock_oom; > } OK, I've missed this part. So you just defer the decision to the oom reaper while I am trying to solve that at oom_kill_process level. We could very well do diff --git a/mm/oom_kill.c b/mm/oom_kill.c index bcb6d3b26c94..d9017b8c7300 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -813,6 +813,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p, * memory might be still used. */ can_oom_reap = false; + set_bit(MMF_OOM_REAPED, mm->flags); continue; } if (p->signal->oom_score_adj == OOM_ADJUST_MIN) with the same result. If you _really_ think that this would make a difference I could live with that. But I am highly skeptical this matters all that much. > > > Shared with global init is just non > > existant problem. Such a system would be crippled enough to not bother. > > See commit a2b829d95958da20 ("mm/oom_kill.c: avoid attempting to kill init > sharing same memory"). Don't you think that a system where the largest memory consumer is the global init is crippled terribly? -- Michal Hocko SUSE Labs -- 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>