All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Miquel van Smoorenburg <mikevs@xs4all.net>
Cc: Andi Kleen <andi@firstfloor.org>,
	Glauber Costa <gcosta@redhat.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] 2.6.26-rc: x86: pci-dma.c: use __GFP_NO_OOM instead of __GFP_NORETRY
Date: Wed, 28 May 2008 01:40:17 -0700	[thread overview]
Message-ID: <20080528014017.9b3d116f.akpm@linux-foundation.org> (raw)
In-Reply-To: <1211963485.28138.14.camel@localhost.localdomain>

On Wed, 28 May 2008 10:31:25 +0200 Miquel van Smoorenburg <mikevs@xs4all.net> wrote:

> > When the 16MB zone overflows (which can be common in some workloads)
> > calling the OOM killer is pretty useless because it has barely any 
> > real user data [only exception would be the "only 16MB" case Alan
> > mentioned]. Killing random processes in this case is bad. 
> > 
> > I think for 16MB __GFP_NORETRY is ok because there should be 
> > nothing freeable in there so looping is useless. Only exception would be the 
> > "only 16MB total" case again but I'm not sure 2.6 supports that at all
> > on x86.
> > 
> > On the other hand d_a_c() does more allocations than just 16MB, especially
> > on 64bit and the other zones need different strategies.
> 
> Okay, so how about this then ?
> 
> --- linux-2.6.26-rc4.orig/arch/x86/kernel/pci-dma.c	2008-05-26 20:08:11.000000000 +0200
> +++ linux-2.6.26-rc4/arch/x86/kernel/pci-dma.c	2008-05-28 10:27:41.000000000 +0200
> @@ -397,9 +397,6 @@
>  	if (dev->dma_mask == NULL)
>  		return NULL;
>  
> -	/* Don't invoke OOM killer */
> -	gfp |= __GFP_NORETRY;
> -
>  #ifdef CONFIG_X86_64
>  	/* Why <=? Even when the mask is smaller than 4GB it is often
>  	   larger than 16MB and in this case we have a chance of
> @@ -410,7 +407,9 @@
>  #endif
>  
>   again:
> -	page = dma_alloc_pages(dev, gfp, get_order(size));
> +	/* Don't invoke OOM killer or retry in lower 16MB DMA zone */
> +	page = dma_alloc_pages(dev,
> +		(gfp & GFP_DMA) ? gfp | __GFP_NORETRY : gfp, get_order(size));
>  	if (page == NULL)
>  		return NULL;

I guess that's more specifally solving that-which-we-wish-to-solve.

Formally we should be testing __GFP_DMA here, not GFP_DMA - just the
zone selector field.  They're presently equal, but someone could
legitimately come along and do

#define GFP_DMA (__GFP_DMA|__GFP_HIGH)

or similar.

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Miquel van Smoorenburg <mikevs@xs4all.net>
Cc: Andi Kleen <andi@firstfloor.org>,
	Glauber Costa <gcosta@redhat.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH] 2.6.26-rc: x86: pci-dma.c: use __GFP_NO_OOM instead of __GFP_NORETRY
Date: Wed, 28 May 2008 01:40:17 -0700	[thread overview]
Message-ID: <20080528014017.9b3d116f.akpm@linux-foundation.org> (raw)
In-Reply-To: <1211963485.28138.14.camel@localhost.localdomain>

On Wed, 28 May 2008 10:31:25 +0200 Miquel van Smoorenburg <mikevs@xs4all.net> wrote:

> > When the 16MB zone overflows (which can be common in some workloads)
> > calling the OOM killer is pretty useless because it has barely any 
> > real user data [only exception would be the "only 16MB" case Alan
> > mentioned]. Killing random processes in this case is bad. 
> > 
> > I think for 16MB __GFP_NORETRY is ok because there should be 
> > nothing freeable in there so looping is useless. Only exception would be the 
> > "only 16MB total" case again but I'm not sure 2.6 supports that at all
> > on x86.
> > 
> > On the other hand d_a_c() does more allocations than just 16MB, especially
> > on 64bit and the other zones need different strategies.
> 
> Okay, so how about this then ?
> 
> --- linux-2.6.26-rc4.orig/arch/x86/kernel/pci-dma.c	2008-05-26 20:08:11.000000000 +0200
> +++ linux-2.6.26-rc4/arch/x86/kernel/pci-dma.c	2008-05-28 10:27:41.000000000 +0200
> @@ -397,9 +397,6 @@
>  	if (dev->dma_mask == NULL)
>  		return NULL;
>  
> -	/* Don't invoke OOM killer */
> -	gfp |= __GFP_NORETRY;
> -
>  #ifdef CONFIG_X86_64
>  	/* Why <=? Even when the mask is smaller than 4GB it is often
>  	   larger than 16MB and in this case we have a chance of
> @@ -410,7 +407,9 @@
>  #endif
>  
>   again:
> -	page = dma_alloc_pages(dev, gfp, get_order(size));
> +	/* Don't invoke OOM killer or retry in lower 16MB DMA zone */
> +	page = dma_alloc_pages(dev,
> +		(gfp & GFP_DMA) ? gfp | __GFP_NORETRY : gfp, get_order(size));
>  	if (page == NULL)
>  		return NULL;

I guess that's more specifally solving that-which-we-wish-to-solve.

Formally we should be testing __GFP_DMA here, not GFP_DMA - just the
zone selector field.  They're presently equal, but someone could
legitimately come along and do

#define GFP_DMA (__GFP_DMA|__GFP_HIGH)

or similar.

--
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:[~2008-05-28  8:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-26 23:49 [PATCH] 2.6.26-rc: x86: pci-dma.c: use __GFP_NO_OOM instead of __GFP_NORETRY Miquel van Smoorenburg
2008-05-26 23:49 ` Miquel van Smoorenburg
2008-05-27  8:03 ` Ingo Molnar
2008-05-27  8:03   ` Ingo Molnar
2008-05-27  8:47 ` Andrew Morton
2008-05-27  8:47   ` Andrew Morton
2008-05-27  9:35   ` Miquel van Smoorenburg
2008-05-27  9:35     ` Miquel van Smoorenburg
2008-05-28  2:47   ` Andi Kleen
2008-05-28  2:47     ` Andi Kleen
2008-05-28  8:31     ` Miquel van Smoorenburg
2008-05-28  8:31       ` Miquel van Smoorenburg
2008-05-28  8:40       ` Andrew Morton [this message]
2008-05-28  8:40         ` Andrew Morton
2008-05-28 12:54         ` Andi Kleen
2008-05-28 12:54           ` Andi Kleen
2008-06-02 10:15       ` Ingo Molnar
2008-06-02 10:15         ` Ingo Molnar
     [not found]         ` <1212682484.4332.7.camel@n2o.xs4all.nl>
2008-06-10 10:23           ` Ingo Molnar
2008-06-10 17:14             ` Jesse Barnes
2008-06-11 14:26               ` Miquel van Smoorenburg
2008-06-26 11:38               ` Ingo Molnar
2008-07-02  2:39                 ` Jesse Barnes
2008-07-02  2:46                   ` Yinghai Lu
2008-07-02  2:48                     ` Jesse Barnes

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=20080528014017.9b3d116f.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=gcosta@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mikevs@xs4all.net \
    --cc=mingo@elte.hu \
    /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.