[PATCH] mm, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[PATCH] mm, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Tetsuo Handa]

We are using warn_alloc() for reporting both allocation failures and
allocation stalls. If we add debug_guardpage_minorder=1 parameter,
all allocation failure and allocation stall reports become pointless
like below. (Below output would be an OOM livelock where all __GFP_FS
allocations got stuck at too_many_isolated() in shrink_inactive_list()
waiting for kswapd, kswapd is waiting for !__GFP_FS allocations, and
all !__GFP_FS allocations did not get stuck at too_many_isolated() in
shrink_inactive_list() but are unable to invoke the OOM killer.)

----------
[    0.000000] Linux version 4.11.0-rc6-next-20170410 (root@ccsecurity) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-11) (GCC) ) #578 SMP Mon Apr 10 23:08:53 JST 2017
[    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0-rc6-next-20170410 (...snipped...) debug_guardpage_minorder=1
(...snipped...)
[    0.000000] Setting debug_guardpage_minorder to 1
(...snipped...)
[   99.064207] Out of memory: Kill process 3097 (a.out) score 999 or sacrifice child
[   99.066488] Killed process 3097 (a.out) total-vm:14408kB, anon-rss:84kB, file-rss:36kB, shmem-rss:0kB
[   99.180378] oom_reaper: reaped process 3097 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
[  128.310487] warn_alloc: 266 callbacks suppressed
[  133.445395] warn_alloc: 74 callbacks suppressed
[  138.517471] warn_alloc: 300 callbacks suppressed
[  143.537630] warn_alloc: 34 callbacks suppressed
[  148.610773] warn_alloc: 277 callbacks suppressed
[  153.630652] warn_alloc: 70 callbacks suppressed
[  158.639891] warn_alloc: 217 callbacks suppressed
[  163.687727] warn_alloc: 120 callbacks suppressed
[  168.709610] warn_alloc: 252 callbacks suppressed
[  173.714659] warn_alloc: 103 callbacks suppressed
[  178.730858] warn_alloc: 248 callbacks suppressed
[  183.797587] warn_alloc: 82 callbacks suppressed
[  188.825250] warn_alloc: 238 callbacks suppressed
[  193.832834] warn_alloc: 102 callbacks suppressed
[  198.876409] warn_alloc: 259 callbacks suppressed
[  203.940073] warn_alloc: 102 callbacks suppressed
[  207.620979] sysrq: SysRq : Resetting
----------

Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
changed to check debug_guardpage_minorder() > 0 when reporting allocation
failures. But the patch description seems to lack why we want to check it.
Let's remove that check so that administrators can get some clue by
allowing warn_alloc() to report e.g. GFP_NOFS | __GFP_NOWARN allocations
are stalling.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: Stanislaw Gruszka <sgruszka@redhat.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 32b31d6..5c8cf2a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3154,8 +3154,7 @@ void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...)
 	static DEFINE_RATELIMIT_STATE(nopage_rs, DEFAULT_RATELIMIT_INTERVAL,
 				      DEFAULT_RATELIMIT_BURST);
 
-	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs) ||
-	    debug_guardpage_minorder() > 0)
+	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs))
 		return;
 
 	pr_warn("%s: ", current->comm);
-- 
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Stanislaw Gruszka]

On Tue, Apr 11, 2017 at 08:27:15PM +0900, Tetsuo Handa wrote:
> We are using warn_alloc() for reporting both allocation failures and
> allocation stalls. If we add debug_guardpage_minorder=1 parameter,
> all allocation failure and allocation stall reports become pointless
> like below. (Below output would be an OOM livelock were all __GFP_FS
> allocations got stuck at too_many_isolated() in shrink_inactive_list()
> waiting for kswapd, kswapd is waiting for !__GFP_FS allocations, and
> all !__GFP_FS allocations did not get stuck at too_many_isolated() in
> shrink_inactive_list() but are unable to invoke the OOM killer.)
> 
> ----------
> [    0.000000] Linux version 4.11.0-rc6-next-20170410 (root@ccsecurity) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-11) (GCC) ) #578 SMP Mon Apr 10 23:08:53 JST 2017
> [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0-rc6-next-20170410 (...snipped...) debug_guardpage_minorder=1
> (...snipped...)
> [    0.000000] Setting debug_guardpage_minorder to 1
> (...snipped...)
> [   99.064207] Out of memory: Kill process 3097 (a.out) score 999 or sacrifice child
> [   99.066488] Killed process 3097 (a.out) total-vm:14408kB, anon-rss:84kB, file-rss:36kB, shmem-rss:0kB
> [   99.180378] oom_reaper: reaped process 3097 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
> [  128.310487] warn_alloc: 266 callbacks suppressed
> [  133.445395] warn_alloc: 74 callbacks suppressed
> [  138.517471] warn_alloc: 300 callbacks suppressed
> [  143.537630] warn_alloc: 34 callbacks suppressed
> [  148.610773] warn_alloc: 277 callbacks suppressed
> [  153.630652] warn_alloc: 70 callbacks suppressed
> [  158.639891] warn_alloc: 217 callbacks suppressed
> [  163.687727] warn_alloc: 120 callbacks suppressed
> [  168.709610] warn_alloc: 252 callbacks suppressed
> [  173.714659] warn_alloc: 103 callbacks suppressed
> [  178.730858] warn_alloc: 248 callbacks suppressed
> [  183.797587] warn_alloc: 82 callbacks suppressed
> [  188.825250] warn_alloc: 238 callbacks suppressed
> [  193.832834] warn_alloc: 102 callbacks suppressed
> [  198.876409] warn_alloc: 259 callbacks suppressed
> [  203.940073] warn_alloc: 102 callbacks suppressed
> [  207.620979] sysrq: SysRq : Resetting
> ----------
> 
> Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
> changed to check debug_guardpage_minorder() > 0 when reporting allocation
> failures. But the patch description seems to lack why we want to check it.

When we use guard page to debug memory corruption, it shrinks available
pages to 1/2, 1/4, 1/8 and so on, depending on parameter value.
In such case memory allocation failures can be common and printing
errors can flood dmesg. If sombody debug corruption, allocation
failures are not the things he/she is interested about.

> Let's remove that check so that administrators can get some clue by
> allowing warn_alloc() to report e.g. GFP_NOFS | __GFP_NOWARN allocations
> are stalling.

This is ok for me, but perhaps move debug_guardpage_minorder() > 0
check before calling warn_alloc() in buddy allocator when it fails,
or move it before __ratelimit(), will be better option.

Thanks
Stanislaw


> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: Stanislaw Gruszka <sgruszka@redhat.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: Christoph Lameter <cl@linux-foundation.org>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  mm/page_alloc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 32b31d6..5c8cf2a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3154,8 +3154,7 @@ void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...)
>  	static DEFINE_RATELIMIT_STATE(nopage_rs, DEFAULT_RATELIMIT_INTERVAL,
>  				      DEFAULT_RATELIMIT_BURST);
>  
> -	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs) ||
> -	    debug_guardpage_minorder() > 0)
> +	if ((gfp_mask & __GFP_NOWARN) || !__ratelimit(&nopage_rs))
>  		return;
>  
>  	pr_warn("%s: ", current->comm);
> -- 
> 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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Tetsuo Handa]

Stanislaw Gruszka wrote:
> On Tue, Apr 11, 2017 at 08:27:15PM +0900, Tetsuo Handa wrote:
> > Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
> > changed to check debug_guardpage_minorder() > 0 when reporting allocation
> > failures. But the patch description seems to lack why we want to check it.
> 
> When we use guard page to debug memory corruption, it shrinks available
> pages to 1/2, 1/4, 1/8 and so on, depending on parameter value.
> In such case memory allocation failures can be common and printing
> errors can flood dmesg. If sombody debug corruption, allocation
> failures are not the things he/she is interested about.

Nowadays we likely have a lot of memory where shrinking available pages to
1/2, 1/4, 1/8 and so on would not cause flooding of allocation failure messages.
Thus, I hope removing debug_guardpage_minorder() > 0 test affects only systems
with small memory. But

> 
> > Let's remove that check so that administrators can get some clue by
> > allowing warn_alloc() to report e.g. GFP_NOFS | __GFP_NOWARN allocations
> > are stalling.
> 
> This is ok for me, but perhaps move debug_guardpage_minorder() > 0
> check before calling warn_alloc() in buddy allocator when it fails,
> or move it before __ratelimit(), will be better option.

before proposing this patch, I proposed a patch at
http://lkml.kernel.org/r/1491825493-8859-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
that ignores debug_guardpage_minorder() > 0 only when reporting allocation stalls.
We can preserve debug_guardpage_minorder() > 0 test if we change to use
a different function for reporting allocation stalls.

Which patch do you prefer?

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Stanislaw Gruszka]

On Wed, Apr 12, 2017 at 07:41:17PM +0900, Tetsuo Handa wrote:
> before proposing this patch, I proposed a patch at
> http://lkml.kernel.org/r/1491825493-8859-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
> that ignores debug_guardpage_minorder() > 0 only when reporting allocation stalls.
> We can preserve debug_guardpage_minorder() > 0 test if we change to use
> a different function for reporting allocation stalls.
> 
> Which patch do you prefer?

I don't have any preferences regarding this.

Stanislaw

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Michal Hocko]

On Wed 12-04-17 12:23:45, Stanislaw Gruszka wrote:
> On Tue, Apr 11, 2017 at 08:27:15PM +0900, Tetsuo Handa wrote:
> > We are using warn_alloc() for reporting both allocation failures and
> > allocation stalls. If we add debug_guardpage_minorder=1 parameter,
> > all allocation failure and allocation stall reports become pointless
> > like below. (Below output would be an OOM livelock were all __GFP_FS
> > allocations got stuck at too_many_isolated() in shrink_inactive_list()
> > waiting for kswapd, kswapd is waiting for !__GFP_FS allocations, and
> > all !__GFP_FS allocations did not get stuck at too_many_isolated() in
> > shrink_inactive_list() but are unable to invoke the OOM killer.)
> > 
> > ----------
> > [    0.000000] Linux version 4.11.0-rc6-next-20170410 (root@ccsecurity) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-11) (GCC) ) #578 SMP Mon Apr 10 23:08:53 JST 2017
> > [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0-rc6-next-20170410 (...snipped...) debug_guardpage_minorder=1
> > (...snipped...)
> > [    0.000000] Setting debug_guardpage_minorder to 1
> > (...snipped...)
> > [   99.064207] Out of memory: Kill process 3097 (a.out) score 999 or sacrifice child
> > [   99.066488] Killed process 3097 (a.out) total-vm:14408kB, anon-rss:84kB, file-rss:36kB, shmem-rss:0kB
> > [   99.180378] oom_reaper: reaped process 3097 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
> > [  128.310487] warn_alloc: 266 callbacks suppressed
> > [  133.445395] warn_alloc: 74 callbacks suppressed
> > [  138.517471] warn_alloc: 300 callbacks suppressed
> > [  143.537630] warn_alloc: 34 callbacks suppressed
> > [  148.610773] warn_alloc: 277 callbacks suppressed
> > [  153.630652] warn_alloc: 70 callbacks suppressed
> > [  158.639891] warn_alloc: 217 callbacks suppressed
> > [  163.687727] warn_alloc: 120 callbacks suppressed
> > [  168.709610] warn_alloc: 252 callbacks suppressed
> > [  173.714659] warn_alloc: 103 callbacks suppressed
> > [  178.730858] warn_alloc: 248 callbacks suppressed
> > [  183.797587] warn_alloc: 82 callbacks suppressed
> > [  188.825250] warn_alloc: 238 callbacks suppressed
> > [  193.832834] warn_alloc: 102 callbacks suppressed
> > [  198.876409] warn_alloc: 259 callbacks suppressed
> > [  203.940073] warn_alloc: 102 callbacks suppressed
> > [  207.620979] sysrq: SysRq : Resetting
> > ----------
> > 
> > Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
> > changed to check debug_guardpage_minorder() > 0 when reporting allocation
> > failures. But the patch description seems to lack why we want to check it.
> 
> When we use guard page to debug memory corruption, it shrinks available
> pages to 1/2, 1/4, 1/8 and so on, depending on parameter value.
> In such case memory allocation failures can be common and printing
> errors can flood dmesg. If sombody debug corruption, allocation
> failures are not the things he/she is interested about.

Can we distinguish those guard page allocations? Why cannot they use
__GFP_NOWARN?
-- 
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Stanislaw Gruszka]

On Wed, Apr 12, 2017 at 12:59:51PM +0200, Michal Hocko wrote:
> On Wed 12-04-17 12:23:45, Stanislaw Gruszka wrote:
> > On Tue, Apr 11, 2017 at 08:27:15PM +0900, Tetsuo Handa wrote:
> > > We are using warn_alloc() for reporting both allocation failures and
> > > allocation stalls. If we add debug_guardpage_minorder=1 parameter,
> > > all allocation failure and allocation stall reports become pointless
> > > like below. (Below output would be an OOM livelock were all __GFP_FS
> > > allocations got stuck at too_many_isolated() in shrink_inactive_list()
> > > waiting for kswapd, kswapd is waiting for !__GFP_FS allocations, and
> > > all !__GFP_FS allocations did not get stuck at too_many_isolated() in
> > > shrink_inactive_list() but are unable to invoke the OOM killer.)
> > > 
> > > ----------
> > > [    0.000000] Linux version 4.11.0-rc6-next-20170410 (root@ccsecurity) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-11) (GCC) ) #578 SMP Mon Apr 10 23:08:53 JST 2017
> > > [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0-rc6-next-20170410 (...snipped...) debug_guardpage_minorder=1
> > > (...snipped...)
> > > [    0.000000] Setting debug_guardpage_minorder to 1
> > > (...snipped...)
> > > [   99.064207] Out of memory: Kill process 3097 (a.out) score 999 or sacrifice child
> > > [   99.066488] Killed process 3097 (a.out) total-vm:14408kB, anon-rss:84kB, file-rss:36kB, shmem-rss:0kB
> > > [   99.180378] oom_reaper: reaped process 3097 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
> > > [  128.310487] warn_alloc: 266 callbacks suppressed
> > > [  133.445395] warn_alloc: 74 callbacks suppressed
> > > [  138.517471] warn_alloc: 300 callbacks suppressed
> > > [  143.537630] warn_alloc: 34 callbacks suppressed
> > > [  148.610773] warn_alloc: 277 callbacks suppressed
> > > [  153.630652] warn_alloc: 70 callbacks suppressed
> > > [  158.639891] warn_alloc: 217 callbacks suppressed
> > > [  163.687727] warn_alloc: 120 callbacks suppressed
> > > [  168.709610] warn_alloc: 252 callbacks suppressed
> > > [  173.714659] warn_alloc: 103 callbacks suppressed
> > > [  178.730858] warn_alloc: 248 callbacks suppressed
> > > [  183.797587] warn_alloc: 82 callbacks suppressed
> > > [  188.825250] warn_alloc: 238 callbacks suppressed
> > > [  193.832834] warn_alloc: 102 callbacks suppressed
> > > [  198.876409] warn_alloc: 259 callbacks suppressed
> > > [  203.940073] warn_alloc: 102 callbacks suppressed
> > > [  207.620979] sysrq: SysRq : Resetting
> > > ----------
> > > 
> > > Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
> > > changed to check debug_guardpage_minorder() > 0 when reporting allocation
> > > failures. But the patch description seems to lack why we want to check it.
> > 
> > When we use guard page to debug memory corruption, it shrinks available
> > pages to 1/2, 1/4, 1/8 and so on, depending on parameter value.
> > In such case memory allocation failures can be common and printing
> > errors can flood dmesg. If sombody debug corruption, allocation
> > failures are not the things he/she is interested about.
> 
> Can we distinguish those guard page allocations?

Allocation failures happen on standard pages, due to limit of available pages.
Because much of pages become unused - guard pages are reserved pages marked
as no-read/no-write (basically this is artificial memory shrink).

>Why cannot they use
> __GFP_NOWARN?

That some option, though I think setting __GFP_NOWARN if debug_guardpage_enabled()
is set, instead of checking that directly make no big difference anyway.

Stanislaw

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Michal Hocko]

On Wed 12-04-17 13:21:58, Stanislaw Gruszka wrote:
> On Wed, Apr 12, 2017 at 12:59:51PM +0200, Michal Hocko wrote:
> > On Wed 12-04-17 12:23:45, Stanislaw Gruszka wrote:
> > > On Tue, Apr 11, 2017 at 08:27:15PM +0900, Tetsuo Handa wrote:
> > > > We are using warn_alloc() for reporting both allocation failures and
> > > > allocation stalls. If we add debug_guardpage_minorder=1 parameter,
> > > > all allocation failure and allocation stall reports become pointless
> > > > like below. (Below output would be an OOM livelock were all __GFP_FS
> > > > allocations got stuck at too_many_isolated() in shrink_inactive_list()
> > > > waiting for kswapd, kswapd is waiting for !__GFP_FS allocations, and
> > > > all !__GFP_FS allocations did not get stuck at too_many_isolated() in
> > > > shrink_inactive_list() but are unable to invoke the OOM killer.)
> > > > 
> > > > ----------
> > > > [    0.000000] Linux version 4.11.0-rc6-next-20170410 (root@ccsecurity) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-11) (GCC) ) #578 SMP Mon Apr 10 23:08:53 JST 2017
> > > > [    0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.11.0-rc6-next-20170410 (...snipped...) debug_guardpage_minorder=1
> > > > (...snipped...)
> > > > [    0.000000] Setting debug_guardpage_minorder to 1
> > > > (...snipped...)
> > > > [   99.064207] Out of memory: Kill process 3097 (a.out) score 999 or sacrifice child
> > > > [   99.066488] Killed process 3097 (a.out) total-vm:14408kB, anon-rss:84kB, file-rss:36kB, shmem-rss:0kB
> > > > [   99.180378] oom_reaper: reaped process 3097 (a.out), now anon-rss:0kB, file-rss:0kB, shmem-rss:0kB
> > > > [  128.310487] warn_alloc: 266 callbacks suppressed
> > > > [  133.445395] warn_alloc: 74 callbacks suppressed
> > > > [  138.517471] warn_alloc: 300 callbacks suppressed
> > > > [  143.537630] warn_alloc: 34 callbacks suppressed
> > > > [  148.610773] warn_alloc: 277 callbacks suppressed
> > > > [  153.630652] warn_alloc: 70 callbacks suppressed
> > > > [  158.639891] warn_alloc: 217 callbacks suppressed
> > > > [  163.687727] warn_alloc: 120 callbacks suppressed
> > > > [  168.709610] warn_alloc: 252 callbacks suppressed
> > > > [  173.714659] warn_alloc: 103 callbacks suppressed
> > > > [  178.730858] warn_alloc: 248 callbacks suppressed
> > > > [  183.797587] warn_alloc: 82 callbacks suppressed
> > > > [  188.825250] warn_alloc: 238 callbacks suppressed
> > > > [  193.832834] warn_alloc: 102 callbacks suppressed
> > > > [  198.876409] warn_alloc: 259 callbacks suppressed
> > > > [  203.940073] warn_alloc: 102 callbacks suppressed
> > > > [  207.620979] sysrq: SysRq : Resetting
> > > > ----------
> > > > 
> > > > Commit c0a32fc5a2e470d0 ("mm: more intensive memory corruption debugging")
> > > > changed to check debug_guardpage_minorder() > 0 when reporting allocation
> > > > failures. But the patch description seems to lack why we want to check it.
> > > 
> > > When we use guard page to debug memory corruption, it shrinks available
> > > pages to 1/2, 1/4, 1/8 and so on, depending on parameter value.
> > > In such case memory allocation failures can be common and printing
> > > errors can flood dmesg. If sombody debug corruption, allocation
> > > failures are not the things he/she is interested about.
> > 
> > Can we distinguish those guard page allocations?
> 
> Allocation failures happen on standard pages, due to limit of available pages.
> Because much of pages become unused - guard pages are reserved pages marked
> as no-read/no-write (basically this is artificial memory shrink).

OK, I see. That is a rather weird feature and the naming is more than
surprising. But put that aside. Then it means that the check should be
pulled out to 
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6632256ef170..1e5f3b5cdb87 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
 		goto retry;
 	}
 fail:
-	warn_alloc(gfp_mask, ac->nodemask,
+	if (!debug_guardpage_minorder())
+		warn_alloc(gfp_mask, ac->nodemask,
 			"page allocation failure: order:%u", order);
 got_pg:
 	return page;
-- 
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Stanislaw Gruszka]

On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> OK, I see. That is a rather weird feature and the naming is more than
> surprising. But put that aside. Then it means that the check should be
> pulled out to 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6632256ef170..1e5f3b5cdb87 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  		goto retry;
>  	}
>  fail:
> -	warn_alloc(gfp_mask, ac->nodemask,
> +	if (!debug_guardpage_minorder())
> +		warn_alloc(gfp_mask, ac->nodemask,
>  			"page allocation failure: order:%u", order);
>  got_pg:
>  	return page;

Looks good to me assuming it will be applied on top of Tetsuo's patch.

Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Michal Hocko]

On Wed 12-04-17 13:48:16, Stanislaw Gruszka wrote:
> On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> > OK, I see. That is a rather weird feature and the naming is more than
> > surprising. But put that aside. Then it means that the check should be
> > pulled out to 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 6632256ef170..1e5f3b5cdb87 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >  		goto retry;
> >  	}
> >  fail:
> > -	warn_alloc(gfp_mask, ac->nodemask,
> > +	if (!debug_guardpage_minorder())
> > +		warn_alloc(gfp_mask, ac->nodemask,
> >  			"page allocation failure: order:%u", order);
> >  got_pg:
> >  	return page;
> 
> Looks good to me assuming it will be applied on top of Tetsuo's patch.

This also asks for a comment explaining why debug_guardpage_minorder is
special. Your previous clarification should be OK.
 
> Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>

Tetsuo care to send an updated patch?
-- 
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Tetsuo Handa]

Stanislaw Gruszka wrote:
> On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> > OK, I see. That is a rather weird feature and the naming is more than
> > surprising. But put that aside. Then it means that the check should be
> > pulled out to 
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 6632256ef170..1e5f3b5cdb87 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >  		goto retry;
> >  	}
> >  fail:
> > -	warn_alloc(gfp_mask, ac->nodemask,
> > +	if (!debug_guardpage_minorder())
> > +		warn_alloc(gfp_mask, ac->nodemask,
> >  			"page allocation failure: order:%u", order);
> >  got_pg:
> >  	return page;
> 
> Looks good to me assuming it will be applied on top of Tetsuo's patch.
> 
> Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> 

There are two warn_alloc() usages in mm/vmalloc.c which the check should be
pulled out. Then, I feel changing to use a different function for reporting
allocation stalls might be better than pulling out

	if (!debug_guardpage_minorder())

into three locations.

Michal, you can fold my patch into your patch if you prefer pulling out.

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Michal Hocko]

On Wed 12-04-17 21:14:10, Tetsuo Handa wrote:
> Stanislaw Gruszka wrote:
> > On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> > > OK, I see. That is a rather weird feature and the naming is more than
> > > surprising. But put that aside. Then it means that the check should be
> > > pulled out to 
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 6632256ef170..1e5f3b5cdb87 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> > >  		goto retry;
> > >  	}
> > >  fail:
> > > -	warn_alloc(gfp_mask, ac->nodemask,
> > > +	if (!debug_guardpage_minorder())
> > > +		warn_alloc(gfp_mask, ac->nodemask,
> > >  			"page allocation failure: order:%u", order);
> > >  got_pg:
> > >  	return page;
> > 
> > Looks good to me assuming it will be applied on top of Tetsuo's patch.
> > 
> > Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > 
> 
> There are two warn_alloc() usages in mm/vmalloc.c which the check should be
> pulled out.

Do we actually care about vmalloc for this?
-- 
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Tetsuo Handa]

Michal Hocko wrote:
> On Wed 12-04-17 21:14:10, Tetsuo Handa wrote:
> > Stanislaw Gruszka wrote:
> > > On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> > > > OK, I see. That is a rather weird feature and the naming is more than
> > > > surprising. But put that aside. Then it means that the check should be
> > > > pulled out to 
> > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > > index 6632256ef170..1e5f3b5cdb87 100644
> > > > --- a/mm/page_alloc.c
> > > > +++ b/mm/page_alloc.c
> > > > @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> > > >  		goto retry;
> > > >  	}
> > > >  fail:
> > > > -	warn_alloc(gfp_mask, ac->nodemask,
> > > > +	if (!debug_guardpage_minorder())
> > > > +		warn_alloc(gfp_mask, ac->nodemask,
> > > >  			"page allocation failure: order:%u", order);
> > > >  got_pg:
> > > >  	return page;
> > > 
> > > Looks good to me assuming it will be applied on top of Tetsuo's patch.
> > > 
> > > Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > > 
> > 
> > There are two warn_alloc() usages in mm/vmalloc.c which the check should be
> > pulled out.
> 
> Do we actually care about vmalloc for this?

Does it make sense not to apply debug_guardpage_minorder() > 0 test when
kmalloc() path in kvmalloc() failed due to out of available pages and
vmalloc() fallback path again failed due to out of available pages?

If the purpose of debug_guardpage_minorder() > 0 test is to prevent from flooding
allocation failure messages, why not to treat kmalloc()/vmalloc() evenly?

Yes, we might think it is better to print allocation failure messages if memory is
tight enough to even vmalloc() fails. But this patch's intention is to make sure
that allocation stall messages are not disabled by debug_guardpage_minorder() > 0
test. I guess this patch should not change behavior of allocation failure messages.

--
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, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().

[Michal Hocko]

On Wed 12-04-17 22:49:22, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Wed 12-04-17 21:14:10, Tetsuo Handa wrote:
> > > Stanislaw Gruszka wrote:
> > > > On Wed, Apr 12, 2017 at 01:35:28PM +0200, Michal Hocko wrote:
> > > > > OK, I see. That is a rather weird feature and the naming is more than
> > > > > surprising. But put that aside. Then it means that the check should be
> > > > > pulled out to 
> > > > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > > > index 6632256ef170..1e5f3b5cdb87 100644
> > > > > --- a/mm/page_alloc.c
> > > > > +++ b/mm/page_alloc.c
> > > > > @@ -3941,7 +3941,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> > > > >  		goto retry;
> > > > >  	}
> > > > >  fail:
> > > > > -	warn_alloc(gfp_mask, ac->nodemask,
> > > > > +	if (!debug_guardpage_minorder())
> > > > > +		warn_alloc(gfp_mask, ac->nodemask,
> > > > >  			"page allocation failure: order:%u", order);
> > > > >  got_pg:
> > > > >  	return page;
> > > > 
> > > > Looks good to me assuming it will be applied on top of Tetsuo's patch.
> > > > 
> > > > Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > > > 
> > > 
> > > There are two warn_alloc() usages in mm/vmalloc.c which the check should be
> > > pulled out.
> > 
> > Do we actually care about vmalloc for this?
> 
> Does it make sense not to apply debug_guardpage_minorder() > 0 test when
> kmalloc() path in kvmalloc() failed due to out of available pages and
> vmalloc() fallback path again failed due to out of available pages?

Well, vmalloc warns in 2 situations
	- when the order-0 page allocation fails
	- when the vmalloc area fails which can be either due to memory
	  allocation failure or because of vmalloc space depletion for
	  the given size when debug_guardpage_minorder is mostly
	  irrelevant

considering that we are talking about small allocations, mostly
GFP_KERNEL compatible, none of this should cause any warning floods in
the kernel log. So I do not really think they should care about
debug_guardpage_minorder at all.

In fact the more I think about this the more I am convinced that the
whole debug_guardpage_minorder check is just pointless. Because small
allocations would simply go OOM and we would flood the log anyway and
large order allocations are not all that common to actually matter. So,
let me ask again, is this something that is a result of a real problem
showing up with the guardpage or whatever is the name of the debugging
feature, or this is more a just in case thing?

-- 
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>