All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: David Rientjes <rientjes@google.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [PATCH 2/2] print vmalloc() state after allocation failures
Date: Fri, 08 Apr 2011 13:47:14 -0700	[thread overview]
Message-ID: <1302295634.7286.1146.camel@nimitz> (raw)
In-Reply-To: <alpine.DEB.2.00.1104081337470.12689@chino.kir.corp.google.com>

On Fri, 2011-04-08 at 13:39 -0700, David Rientjes wrote:
> On Fri, 8 Apr 2011, Dave Hansen wrote:
> > This patch will print out messages that look like this:
> > 
> > [   30.040774] bash: vmalloc failure allocating after 0 / 73728 bytes
> > 
> 
> Either the changelog or the patch is still wrong because the format of 
> this string is inconsistent.

Yeah, ya caught me. :)
> > diff -puN mm/vmalloc.c~vmalloc-warn mm/vmalloc.c
> > --- linux-2.6.git/mm/vmalloc.c~vmalloc-warn	2011-04-08 09:36:05.877020199 -0700
> > +++ linux-2.6.git-dave/mm/vmalloc.c	2011-04-08 09:38:00.373093593 -0700
> > @@ -1534,6 +1534,7 @@ static void *__vmalloc_node(unsigned lon
> >  static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> >  				 pgprot_t prot, int node, void *caller)
> >  {
> > +	int order = 0;
> 
> Unnecessary, we can continue to hardcode the 0, vmalloc isn't going to use 
> higher order allocs (it's there to avoid such things!).

The only reason I did that was to keep the printk from looking like
this:

> > +	nopage_warning(gfp_mask, 0,  "vmalloc: allocation failure, "
> > +			"allocated %ld of %ld bytes\n",
> > +			(area->nr_pages*PAGE_SIZE), area->size);

The order is pretty darn obvious in the direct allocator calls, but I
liked having it named where it wasn't as obvious.

> >  	struct page **pages;
> >  	unsigned int nr_pages, array_size, i;
> >  	gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO;
> > @@ -1560,11 +1561,12 @@ static void *__vmalloc_area_node(struct 
> >  
> >  	for (i = 0; i < area->nr_pages; i++) {
> >  		struct page *page;
> > +		gfp_t tmp_mask = gfp_mask | __GFP_NOWARN;
> 
> I think it would be better to just do away with this as well and just 
> hardwire the __GFP_NOWARN directly into the two allocation calls.

I did it because hard-wiring it takes the alloc_pages_node() one over 80
columns.  I figured if I was going to add a line, I might as well keep
it pretty.

-- Dave


WARNING: multiple messages have this Message-ID (diff)
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: David Rientjes <rientjes@google.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Johannes Weiner <hannes@cmpxchg.org>
Subject: Re: [PATCH 2/2] print vmalloc() state after allocation failures
Date: Fri, 08 Apr 2011 13:47:14 -0700	[thread overview]
Message-ID: <1302295634.7286.1146.camel@nimitz> (raw)
In-Reply-To: <alpine.DEB.2.00.1104081337470.12689@chino.kir.corp.google.com>

On Fri, 2011-04-08 at 13:39 -0700, David Rientjes wrote:
> On Fri, 8 Apr 2011, Dave Hansen wrote:
> > This patch will print out messages that look like this:
> > 
> > [   30.040774] bash: vmalloc failure allocating after 0 / 73728 bytes
> > 
> 
> Either the changelog or the patch is still wrong because the format of 
> this string is inconsistent.

Yeah, ya caught me. :)
> > diff -puN mm/vmalloc.c~vmalloc-warn mm/vmalloc.c
> > --- linux-2.6.git/mm/vmalloc.c~vmalloc-warn	2011-04-08 09:36:05.877020199 -0700
> > +++ linux-2.6.git-dave/mm/vmalloc.c	2011-04-08 09:38:00.373093593 -0700
> > @@ -1534,6 +1534,7 @@ static void *__vmalloc_node(unsigned lon
> >  static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> >  				 pgprot_t prot, int node, void *caller)
> >  {
> > +	int order = 0;
> 
> Unnecessary, we can continue to hardcode the 0, vmalloc isn't going to use 
> higher order allocs (it's there to avoid such things!).

The only reason I did that was to keep the printk from looking like
this:

> > +	nopage_warning(gfp_mask, 0,  "vmalloc: allocation failure, "
> > +			"allocated %ld of %ld bytes\n",
> > +			(area->nr_pages*PAGE_SIZE), area->size);

The order is pretty darn obvious in the direct allocator calls, but I
liked having it named where it wasn't as obvious.

> >  	struct page **pages;
> >  	unsigned int nr_pages, array_size, i;
> >  	gfp_t nested_gfp = (gfp_mask & GFP_RECLAIM_MASK) | __GFP_ZERO;
> > @@ -1560,11 +1561,12 @@ static void *__vmalloc_area_node(struct 
> >  
> >  	for (i = 0; i < area->nr_pages; i++) {
> >  		struct page *page;
> > +		gfp_t tmp_mask = gfp_mask | __GFP_NOWARN;
> 
> I think it would be better to just do away with this as well and just 
> hardwire the __GFP_NOWARN directly into the two allocation calls.

I did it because hard-wiring it takes the alloc_pages_node() one over 80
columns.  I figured if I was going to add a line, I might as well keep
it pretty.

-- Dave

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-04-08 20:47 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-08 20:22 [PATCH 1/2] break out page allocation warning code Dave Hansen
2011-04-08 20:22 ` Dave Hansen
2011-04-08 20:22 ` [PATCH 2/2] print vmalloc() state after allocation failures Dave Hansen
2011-04-08 20:22   ` Dave Hansen
2011-04-08 20:39   ` David Rientjes
2011-04-08 20:39     ` David Rientjes
2011-04-08 20:47     ` Dave Hansen [this message]
2011-04-08 20:47       ` Dave Hansen
2011-04-08 20:37 ` [PATCH 1/2] break out page allocation warning code David Rientjes
2011-04-08 20:37   ` David Rientjes
2011-04-08 20:43   ` Dave Hansen
2011-04-08 20:43     ` Dave Hansen
2011-04-08 20:54 ` Michał Nazarewicz
2011-04-08 21:02   ` Dave Hansen
2011-04-08 21:02     ` Dave Hansen
2011-04-11 10:20     ` Michal Nazarewicz
2011-04-11 10:20       ` Michal Nazarewicz
  -- strict thread matches above, loose matches on Subject: below --
2011-04-15 17:04 Dave Hansen
2011-04-15 17:04 ` [PATCH 2/2] print vmalloc() state after allocation failures Dave Hansen
2011-04-15 17:04   ` Dave Hansen
2011-04-15 17:20   ` Michal Nazarewicz
2011-04-15 17:20     ` Michal Nazarewicz
2011-04-15 17:44     ` Dave Hansen
2011-04-15 17:44       ` Dave Hansen
2011-04-17  0:03       ` David Rientjes
2011-04-17  0:03         ` David Rientjes
2011-04-18 15:21         ` Dave Hansen
2011-04-18 15:21           ` Dave Hansen
2011-04-19 16:21 [PATCH 1/2] break out page allocation warning code Dave Hansen
2011-04-19 16:21 ` [PATCH 2/2] print vmalloc() state after allocation failures Dave Hansen
2011-04-19 16:21   ` Dave Hansen

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=1302295634.7286.1146.camel@nimitz \
    --to=dave@linux.vnet.ibm.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.