Re: [PATCH] 2.6.26-rc: x86: pci-dma.c: use __GFP_NO_OOM instead of __GFP_NORETRY

[Ingo Molnar <mingo@elte.hu>]

* Miquel van Smoorenburg <mikevs@xs4all.net> wrote:

> On Mon, 2008-06-02 at 12:15 +0200, Ingo Molnar wrote:
> > * Miquel van Smoorenburg <mikevs@xs4all.net> wrote:
> > 
> > > Okay, so how about this then ?
> >
> > 
> > applied to tip/pci-for-jesse for more testing. Thanks,
> 
> I've thought about it a bit more, and I think the actual patch that 
> really does what everybody wants is this one instead:

applied a delta patch version of the one below to tip/pci-for-jesse. 
Thanks,

	Ingo

> [PATCH] x86: pci-dma.c: don't always add __GFP_NORETRY to gfp
> 
> Currently arch/x86/kernel/pci-dma.c always adds __GFP_NORETRY
> to the allocation flags, because it wants to be reasonably
> sure not to deadlock when calling alloc_pages().
> 
> But really that should only be done in two cases:
> - when allocating memory in the lower 16 MB DMA zone.
>   If there's no free memory there, waiting or OOM killing is of no use
> - when optimistically trying an allocation in the DMA32 zone
>   when dma_mask < DMA_32BIT_MASK hoping that the allocation
>   happens to fall within the limits of the dma_mask
> 
> Also blindly adding __GFP_NORETRY to the the gfp variable might
> not be a good idea since we then also use it when calling
> dma_ops->alloc_coherent(). Clearing it might also not be a
> good idea, dma_alloc_coherent()'s caller might have set it
> on purpose. The gfp variable should not be clobbered.
> 
> Signed-off-by: Miquel van Smoorenburg <miquels@cistron.nl>
> 
> --- 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-06-05 17:51:41.000000000 +0200
> @@ -378,6 +378,7 @@ dma_alloc_coherent(struct device *dev, s
>  	struct page *page;
>  	unsigned long dma_mask = 0;
>  	dma_addr_t bus;
> +	int noretry = 0;
>  
>  	/* ignore region specifiers */
>  	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
> @@ -397,20 +398,25 @@ dma_alloc_coherent(struct device *dev, s
>  	if (dev->dma_mask == NULL)
>  		return NULL;
>  
> -	/* Don't invoke OOM killer */
> -	gfp |= __GFP_NORETRY;
> +	/* Don't invoke OOM killer or retry in lower 16MB DMA zone */
> +	if (gfp & __GFP_DMA)
> +		noretry = 1;
>  
>  #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
>  	   finding fitting memory in the next higher zone first. If
>  	   not retry with true GFP_DMA. -AK */
> -	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA))
> +	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA)) {
>  		gfp |= GFP_DMA32;
> +		if (dma_mask < DMA_32BIT_MASK)
> +			noretry = 1;
> +	}
>  #endif
>  
>   again:
> -	page = dma_alloc_pages(dev, gfp, get_order(size));
> +	page = dma_alloc_pages(dev,
> +		noretry ? gfp | __GFP_NORETRY : gfp, get_order(size));
>  	if (page == NULL)
>  		return NULL;
>  
>