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

[David Rientjes <rientjes@xxxxxxxxxx>]

On Tue, 24 Jul 2018, Michal Hocko wrote:

> 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",
> 
> 
> On top of that the proposed cleanup looks as follows:
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 88657e018714..4e185a282b3d 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -541,8 +541,16 @@ bool __oom_reap_task_mm(struct mm_struct *mm)
>       return ret;
>  }
>  
> +/*
> + * Reaps the address space of the give task.
> + *
> + * Returns true on success and false if none or part of the address space
> + * has been reclaimed and the caller should retry later.
> + */
>  static bool oom_reap_task_mm(struct task_struct *tsk, struct mm_struct *mm)
>  {
> +     bool ret = true;
> +
>       if (!down_read_trylock(&mm->mmap_sem)) {
>               trace_skip_task_reaping(tsk->pid);
>               return false;
> @@ -555,28 +563,28 @@ static bool oom_reap_task_mm(struct task_struct *tsk, 
> struct mm_struct *mm)
>        * down_write();up_write() cycle in exit_mmap().
>        */
>       if (test_bit(MMF_OOM_SKIP, &mm->flags)) {
> -             up_read(&mm->mmap_sem);
>               trace_skip_task_reaping(tsk->pid);
> -             return true;
> +             goto out_unlock;
>       }
>  
>       trace_start_task_reaping(tsk->pid);
>  
>       /* failed to reap part of the address space. Try again later */
> -     if (!__oom_reap_task_mm(mm)) {
> -             up_read(&mm->mmap_sem);
> -             return false;
> -     }
> +     ret = __oom_reap_task_mm(mm);
> +     if (!ret)
> +             goto out_finish;
>  
>       pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, 
> file-rss:%lukB, shmem-rss:%lukB\n",
>                       task_pid_nr(tsk), tsk->comm,
>                       K(get_mm_counter(mm, MM_ANONPAGES)),
>                       K(get_mm_counter(mm, MM_FILEPAGES)),
>                       K(get_mm_counter(mm, MM_SHMEMPAGES)));
> +out_finish:
> +     trace_finish_task_reaping(tsk->pid);
> +out_unlock:
>       up_read(&mm->mmap_sem);
>  
> -     trace_finish_task_reaping(tsk->pid);
> -     return true;
> +     return ret;
>  }
>  
>  #define MAX_OOM_REAP_RETRIES 10

I think we still want to trace when reaping was skipped to know that the 
oom reaper will retry again later.



mm/oom_kill.c: clean up oom_reap_task_mm() fix

indicate reaping has been partially skipped so we can expect future skips 
or another start before finish.

Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
---
 mm/oom_kill.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -569,10 +569,12 @@ static bool oom_reap_task_mm(struct task_struct *tsk, 
struct mm_struct *mm)
 
        trace_start_task_reaping(tsk->pid);
 
-       /* failed to reap part of the address space. Try again later */
        ret = __oom_reap_task_mm(mm);
-       if (!ret)
+       if (!ret) {
+               /* Failed to reap part of the address space. Try again later */
+               trace_skip_task_reaping(tsk->pid);
                goto out_finish;
+       }
 
        pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, 
file-rss:%lukB, shmem-rss:%lukB\n",
                        task_pid_nr(tsk), tsk->comm,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel