Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH 3/3] mm: delete unused MMF_OOM_VICTIM flag
To:     Suren Baghdasaryan <surenb@google.com>, akpm@linux-foundation.org
Cc:     mhocko@suse.com, rientjes@google.com, willy@infradead.org,
        hannes@cmpxchg.org, guro@fb.com, minchan@kernel.org,
        kirill@shutemov.name, aarcange@redhat.com, brauner@kernel.org,
        hch@infradead.org, oleg@redhat.com, david@redhat.com,
        jannh@google.com, shakeelb@google.com, peterx@redhat.com,
        jhubbard@nvidia.com, shuah@kernel.org,
        linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        linux-kselftest@vger.kernel.org, kernel-team@android.com,
        Shuah Khan <skhan@linuxfoundation.org>
References: <20220510030014.3842475-1-surenb@google.com>
 <20220510030014.3842475-3-surenb@google.com>
From:   Shuah Khan <skhan@linuxfoundation.org>
Message-ID: <008de890-7fe1-aeae-345e-0cd3fcd32352@linuxfoundation.org>
Date:   Tue, 10 May 2022 09:51:01 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.1
MIME-Version: 1.0
In-Reply-To: <20220510030014.3842475-3-surenb@google.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/9/22 9:00 PM, Suren Baghdasaryan wrote:
> With the last usage of MMF_OOM_VICTIM in exit_mmap gone, this flag is
> now unused and can be removed.
> 
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
>   include/linux/oom.h            | 9 ---------
>   include/linux/sched/coredump.h | 1 -
>   mm/oom_kill.c                  | 4 +---
>   3 files changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/include/linux/oom.h b/include/linux/oom.h
> index 6cdf0772dbae..25990e9d9e15 100644
> --- 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);
> -}
> -
>   /*
>    * Checks whether a page fault on the given mm is still reliable.
>    * This is no longer true if the oom reaper started to reap the
> diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h
> index 4d9e3a656875..746f6cb07a20 100644
> --- a/include/linux/sched/coredump.h
> +++ b/include/linux/sched/coredump.h
> @@ -70,7 +70,6 @@ static inline int get_dumpable(struct mm_struct *mm)
>   #define MMF_UNSTABLE		22	/* mm is unstable for copy_from_user */
>   #define MMF_HUGE_ZERO_PAGE	23      /* mm has ever used the global huge zero page */
>   #define MMF_DISABLE_THP		24	/* disable THP for all VMAs */
> -#define MMF_OOM_VICTIM		25	/* mm is the oom victim */
>   #define MMF_OOM_REAP_QUEUED	26	/* mm was queued for oom_reaper */
>   #define MMF_MULTIPROCESS	27	/* mm is shared between processes */
>   /*
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 36355b162727..11291b99599f 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -732,10 +732,8 @@ static void mark_oom_victim(struct task_struct *tsk)
>   		return;
>   
>   	/* oom_mm is bound to the signal struct life time. */
> -	if (!cmpxchg(&tsk->signal->oom_mm, NULL, mm)) {
> +	if (!cmpxchg(&tsk->signal->oom_mm, NULL, mm))
>   		mmgrab(tsk->signal->oom_mm);
> -		set_bit(MMF_OOM_VICTIM, &mm->flags);
> -	}
>   
>   	/*
>   	 * Make sure that the task is woken up from uninterruptible sleep
> 

Thank you for working on the new tests and cleanups.

This series needs a cover-letter that explains why this series is needed
that includes the information from this last patch.

Please send v2 with a proper cover letter starting with why this series
is necessary. If you did that, it would have reviewers job is lot easier.

Also it appears you are combining new tests with cleanup patches. I think
patches 2/3 and 3/3 can be a separate series and the new test can be a
separate patch.

thanks,
-- Shuah