[PATCH] mm: Don't emit warning from pagefault_out_of_memory()

[PATCH] mm: Don't emit warning from pagefault_out_of_memory()

[Tetsuo Handa]

Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
to warn when we raced with disabling the OOM killer. But emitting same
backtrace forever after the OOM killer/reaper are disabled is pointless
because the system is already OOM livelocked.

Now, patch "oom, suspend: fix oom_killer_disable vs. pm suspend properly"
introduced a timeout for oom_killer_disable(). Even if we raced with
disabling the OOM killer and the system is OOM livelocked, the OOM killer
will be enabled eventually (in 20 seconds by default) and the OOM livelock
will be solved. Therefore, we no longer need to warn when we raced with
disabling the OOM killer.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/oom_kill.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 0034baf..f284e92 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -1069,16 +1069,6 @@ void pagefault_out_of_memory(void)
 
 	if (!mutex_trylock(&oom_lock))
 		return;
-
-	if (!out_of_memory(&oc)) {
-		/*
-		 * There shouldn't be any user tasks runnable while the
-		 * OOM killer is disabled, so the current task has to
-		 * be a racing OOM victim for which oom_killer_disable()
-		 * is waiting for.
-		 */
-		WARN_ON(test_thread_flag(TIF_MEMDIE));
-	}
-
+	out_of_memory(&oc);
 	mutex_unlock(&oom_lock);
 }
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Don't emit warning from pagefault_out_of_memory()

[Michal Hocko]

On Sat 10-09-16 02:28:40, Tetsuo Handa wrote:
> Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
> raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
> to warn when we raced with disabling the OOM killer. But emitting same
> backtrace forever after the OOM killer/reaper are disabled is pointless
> because the system is already OOM livelocked.

How that would that be forever? Pagefaults are not GFP_NOFAIL and the
killed task would just enter the exit path. Sure we will see one warning
per each g-u-p after that point but the above text seems to be
misleading to me. So can you just drop the last sentence?

> Now, patch "oom, suspend: fix oom_killer_disable vs. pm suspend properly"
> introduced a timeout for oom_killer_disable(). Even if we raced with
> disabling the OOM killer and the system is OOM livelocked, the OOM killer
> will be enabled eventually (in 20 seconds by default) and the OOM livelock
> will be solved. Therefore, we no longer need to warn when we raced with
> disabling the OOM killer.

That being said I guess the warning is really no longer needed as you
say. So I am not against the patch. But the changelog wording seems
misleading to me.

> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> ---
>  mm/oom_kill.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 0034baf..f284e92 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -1069,16 +1069,6 @@ void pagefault_out_of_memory(void)
>  
>  	if (!mutex_trylock(&oom_lock))
>  		return;
> -
> -	if (!out_of_memory(&oc)) {
> -		/*
> -		 * There shouldn't be any user tasks runnable while the
> -		 * OOM killer is disabled, so the current task has to
> -		 * be a racing OOM victim for which oom_killer_disable()
> -		 * is waiting for.
> -		 */
> -		WARN_ON(test_thread_flag(TIF_MEMDIE));
> -	}
> -
> +	out_of_memory(&oc);
>  	mutex_unlock(&oom_lock);
>  }
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Don't emit warning from pagefault_out_of_memory()

[Tetsuo Handa]

Michal Hocko wrote:
> On Sat 10-09-16 02:28:40, Tetsuo Handa wrote:
> > Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
> > raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
> > to warn when we raced with disabling the OOM killer. But emitting same
> > backtrace forever after the OOM killer/reaper are disabled is pointless
> > because the system is already OOM livelocked.
> 
> How that would that be forever? Pagefaults are not GFP_NOFAIL and the
> killed task would just enter the exit path.

Indeed, there is

	/* Avoid allocations with no watermarks from looping endlessly */
	if (test_thread_flag(TIF_MEMDIE) && !(gfp_mask & __GFP_NOFAIL))
		goto nopage;

check.

I don't know if pagefaults can happen after entering do_exit().
If pagefaults can happen after entering do_exit(), can
pagefault_out_of_memory() by PF_EXITING threads make progress
without allocating that page?

>                                             Sure we will see one warning
> per each g-u-p after that point but the above text seems to be
> misleading to me. So can you just drop the last sentence?
> 
> > Now, patch "oom, suspend: fix oom_killer_disable vs. pm suspend properly"
> > introduced a timeout for oom_killer_disable(). Even if we raced with
> > disabling the OOM killer and the system is OOM livelocked, the OOM killer
> > will be enabled eventually (in 20 seconds by default) and the OOM livelock
> > will be solved. Therefore, we no longer need to warn when we raced with
> > disabling the OOM killer.
> 
> That being said I guess the warning is really no longer needed as you
> say. So I am not against the patch. But the changelog wording seems
> misleading to me.

I see. Only patch description was updated.
----------------------------------------
Subject: [PATCH v2] mm: Don't emit warning from pagefault_out_of_memory()
Date: Sat, 10 Sep 2016 02:28:40 +0900
Message-Id: <1473442120-7246-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>

Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
to warn when we raced with disabling the OOM killer.

Now, patch "oom, suspend: fix oom_killer_disable vs. pm suspend properly"
introduced a timeout for oom_killer_disable(). Even if we raced with
disabling the OOM killer and the system is OOM livelocked, the OOM killer
will be enabled eventually (in 20 seconds by default) and the OOM livelock
will be solved. Therefore, we no longer need to warn when we raced with
disabling the OOM killer.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
---
 mm/oom_kill.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 0034baf..f284e92 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -1069,16 +1069,6 @@ void pagefault_out_of_memory(void)
 
 	if (!mutex_trylock(&oom_lock))
 		return;
-
-	if (!out_of_memory(&oc)) {
-		/*
-		 * There shouldn't be any user tasks runnable while the
-		 * OOM killer is disabled, so the current task has to
-		 * be a racing OOM victim for which oom_killer_disable()
-		 * is waiting for.
-		 */
-		WARN_ON(test_thread_flag(TIF_MEMDIE));
-	}
-
+	out_of_memory(&oc);
 	mutex_unlock(&oom_lock);
 }
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Don't emit warning from pagefault_out_of_memory()

[Michal Hocko]

On Mon 12-09-16 20:32:13, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Sat 10-09-16 02:28:40, Tetsuo Handa wrote:
> > > Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
> > > raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
> > > to warn when we raced with disabling the OOM killer. But emitting same
> > > backtrace forever after the OOM killer/reaper are disabled is pointless
> > > because the system is already OOM livelocked.
> > 
> > How that would that be forever? Pagefaults are not GFP_NOFAIL and the
> > killed task would just enter the exit path.
> 
> Indeed, there is
> 
> 	/* Avoid allocations with no watermarks from looping endlessly */
> 	if (test_thread_flag(TIF_MEMDIE) && !(gfp_mask & __GFP_NOFAIL))
> 		goto nopage;
> 
> check.
> 
> I don't know if pagefaults can happen after entering do_exit().

It can via g-u-p but callers should be able to handle the failure.
[...]
> Subject: [PATCH v2] mm: Don't emit warning from pagefault_out_of_memory()
> Date: Sat, 10 Sep 2016 02:28:40 +0900
> Message-Id: <1473442120-7246-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
> 
> Commit c32b3cbe0d067a9c ("oom, PM: make OOM detection in the freezer path
> raceless") inserted a WARN_ON() into pagefault_out_of_memory() in order
> to warn when we raced with disabling the OOM killer.
> 
> Now, patch "oom, suspend: fix oom_killer_disable vs. pm suspend properly"
> introduced a timeout for oom_killer_disable(). Even if we raced with
> disabling the OOM killer and the system is OOM livelocked, the OOM killer
> will be enabled eventually (in 20 seconds by default) and the OOM livelock
> will be solved. Therefore, we no longer need to warn when we raced with
> disabling the OOM killer.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
>  mm/oom_kill.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 0034baf..f284e92 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -1069,16 +1069,6 @@ void pagefault_out_of_memory(void)
>  
>  	if (!mutex_trylock(&oom_lock))
>  		return;
> -
> -	if (!out_of_memory(&oc)) {
> -		/*
> -		 * There shouldn't be any user tasks runnable while the
> -		 * OOM killer is disabled, so the current task has to
> -		 * be a racing OOM victim for which oom_killer_disable()
> -		 * is waiting for.
> -		 */
> -		WARN_ON(test_thread_flag(TIF_MEMDIE));
> -	}
> -
> +	out_of_memory(&oc);
>  	mutex_unlock(&oom_lock);
>  }
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>