All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: sgruszka@redhat.com, akpm@linux-foundation.org,
	linux-mm@kvack.org, rjw@rjwysocki.net, aarcange@redhat.com,
	cl@linux-foundation.org, mgorman@suse.de, penberg@cs.helsinki.fi
Subject: Re: [PATCH] mm, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().
Date: Wed, 12 Apr 2017 16:07:43 +0200	[thread overview]
Message-ID: <20170412140742.GH7157@dhcp22.suse.cz> (raw)
In-Reply-To: <201704122249.CJC39594.SOFJOFVOtHMQFL@I-love.SAKURA.ne.jp>

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>

      reply	other threads:[~2017-04-12 14:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-11 11:27 Tetsuo Handa
2017-04-12 10:23 ` Stanislaw Gruszka
2017-04-12 10:41   ` Tetsuo Handa
2017-04-12 11:08     ` Stanislaw Gruszka
2017-04-12 10:59   ` Michal Hocko
2017-04-12 11:21     ` Stanislaw Gruszka
2017-04-12 11:35       ` Michal Hocko
2017-04-12 11:48         ` Stanislaw Gruszka
2017-04-12 12:12           ` Michal Hocko
2017-04-12 12:14           ` Tetsuo Handa
2017-04-12 12:30             ` Michal Hocko
2017-04-12 13:49               ` Tetsuo Handa
2017-04-12 14:07                 ` Michal Hocko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170412140742.GH7157@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=penberg@cs.helsinki.fi \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rjw@rjwysocki.net \
    --cc=sgruszka@redhat.com \
    --subject='Re: [PATCH] mm, page_alloc: Remove debug_guardpage_minorder() test in warn_alloc().' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.