Re: [PATCH] mm, oom: distinguish blockable mode for mmu notifiers

Andrew Morton
2018-07-24T19:53:07Z

On Tue, 24 Jul 2018 16:17:47 +0200 Michal Hocko <mhocko@kernel.org> wrote:

> On Fri 20-07-18 17:09:02, Andrew Morton wrote:
> [...]
> > - Undocumented return value.
> >
> > - comment "failed to reap part..." is misleading - sounds like it's
> > referring to something which happened in the past, is in fact
> > referring to something which might happen in the future.
> >
> > - fails to call trace_finish_task_reaping() in one case
> >
> > - code duplication.
> >
> > - Increases mmap_sem hold time a little by moving
> > trace_finish_task_reaping() inside the locked region. So sue me ;)
> >
> > - Sharing the finish: path means that the trace event won't
> > distinguish between the two sources of finishing.
> >
> > Please take a look?
>
> oom_reap_task_mm should return false when __oom_reap_task_mm return
> false. This is what my patch did but it seems this changed by
> http://www.ozlabs.org/~akpm/mmotm/broken-out/mm-oom-remove-oom_lock-from-oom_reaper.patch
> so that one should be fixed.
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 104ef4a01a55..88657e018714 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -565,7 +565,7 @@ static bool oom_reap_task_mm(struct task_struct *tsk, struct mm_struct *mm)
> /* failed to reap part of the address space. Try again later */
> if (!__oom_reap_task_mm(mm)) {
> up_read(&mm->mmap_sem);
> - return true;
> + return false;
> }
>
> pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",

OK, thanks, I added that.

>
> On top of that the proposed cleanup looks as follows:
>

Looks good to me. Seems a bit strange that we omit the pr_info()
output if the mm was partially reaped - people would still want to know
this? Not very important though.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org