Re: [PATCH RESEND v2 2/2] mm: delete unused MMF_OOM_VICTIM flag

[Yu Zhao]

On Mon, Aug 22, 2022 at 4:21 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 31 May 2022 15:31:00 -0700 Suren Baghdasaryan <surenb@xxxxxxxxxx> wrote:
>
> > With the last usage of MMF_OOM_VICTIM in exit_mmap gone, this flag is
> > now unused and can be removed.
> >
> > ...
> >
> > --- a/include/linux/oom.h
> > +++ b/include/linux/oom.h
> > @@ -77,15 +77,6 @@ static inline bool tsk_is_oom_victim(struct task_struct * tsk)
> >       return tsk->signal->oom_mm;
> >  }
> >
> > -/*
> > - * Use this helper if tsk->mm != mm and the victim mm needs a special
> > - * handling. This is guaranteed to stay true after once set.
> > - */
> > -static inline bool mm_is_oom_victim(struct mm_struct *mm)
> > -{
> > -     return test_bit(MMF_OOM_VICTIM, &mm->flags);
> > -}
> > -
>
> The patch "mm: multi-gen LRU: support page table walks" from the MGLRU
> series
> (https://lkml.kernel.org/r/20220815071332.627393-9-yuzhao@xxxxxxxxxx)
> adds two calls to mm_is_oom_victim(), so my build broke.
>
> I assume the fix is simply
>
> --- a/mm/vmscan.c~mm-delete-unused-mmf_oom_victim-flag-fix
> +++ a/mm/vmscan.c
> @@ -3429,9 +3429,6 @@ static bool should_skip_mm(struct mm_str
>         if (size < MIN_LRU_BATCH)
>                 return true;
>
> -       if (mm_is_oom_victim(mm))
> -               return true;
> -
>         return !mmget_not_zero(mm);
>  }
>
> @@ -4127,9 +4124,6 @@ restart:
>
>                 walk_pmd_range(&val, addr, next, args);
>
> -               if (mm_is_oom_victim(args->mm))
> -                       return 1;
> -
>                 /* a racy check to curtail the waiting time */
>                 if (wq_has_sleeper(&walk->lruvec->mm_state.wait))
>                         return 1;
> _
>
> Please confirm?

LGTM.  The deleted checks are not about correctness.

I've queued

--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3402,7 +3402,7 @@ static bool should_skip_mm(struct mm_struct *mm,
struct lru_gen_mm_walk *walk)
        if (size < MIN_LRU_BATCH)
                return true;

-       if (mm_is_oom_victim(mm))
+       if (test_bit(MMF_OOM_REAP_QUEUED, &mm->flags))
                return true;

        return !mmget_not_zero(mm);
@@ -4109,7 +4109,7 @@ static int walk_pud_range(p4d_t *p4d, unsigned
long start, unsigned long end,

                walk_pmd_range(&val, addr, next, args);

-               if (mm_is_oom_victim(args->mm))
+               if (test_bit(MMF_OOM_REAP_QUEUED, &args->mm->flags))
                        return 1;

                /* a racy check to curtail the waiting time */