Home » Mailing list archives » linux-kernel » 2008 » September » 6

Re: [PATCH] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu

Previous thread: none

Next thread: linux-next: Tree for September 5 by Stephen Rothwell on Friday, September 5, 2008 - 2:09 am. (4 messages)
[view original message]
From: FUJITA Tomonori
Subject: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Friday, September 5, 2008 - 1:58 am

This patchset restores some of the current alloc_coherent behaviors
that Joerg's x86 patchset (in tip/x86/iommu) changes.

The first patch uses __GFP_DMA for NULL device argument (fallback_dev)
with pci-nommu. It's a hack for ISA (and some old code) so we need
DMA_ZONE.

The second patch uses __GFP_NORETRY in the case of GFP_DMA.

The third patch is a minor cleanup.


I'll send patchset for the rest of alloc_coherent issues this weekend.


--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: [PATCH] x86: fix nommu_alloc_coherent allocation with NULL device argument
Date: Friday, September 5, 2008 - 1:58 am

We need to use __GFP_DMA for NULL device argument (fallback_dev) with
pci-nommu. It's a hack for ISA (and some old code) so we need to use
GFP_DMA.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-nommu.c   |    5 +----
 include/asm-x86/dma-mapping.h |    2 ++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index 73853d3..526b2db 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -83,7 +83,6 @@ nommu_alloc_coherent(struct device *hwdev, size_t size,
 	if (hwdev->dma_mask == NULL)
 		return NULL;
 
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
 	gfp |= __GFP_ZERO;
 
 	dma_mask = hwdev->coherent_dma_mask;
@@ -96,14 +95,12 @@ nommu_alloc_coherent(struct device *hwdev, size_t size,
 	node = dev_to_node(hwdev);
 
 #ifdef CONFIG_X86_64
-	if (dma_mask <= DMA_32BIT_MASK)
+	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA))
 		gfp |= GFP_DMA32;
 #endif
-
 	/* No alloc-free penalty for ISA devices */
 	if (dma_mask == DMA_24BIT_MASK)
 		gfp |= GFP_DMA;
-
 again:
 	page = alloc_pages_node(node, gfp, get_order(size));
 	if (!page)
diff --git a/include/asm-x86/dma-mapping.h b/include/asm-x86/dma-mapping.h
index 3a9a6f5..de4495e 100644
--- a/include/asm-x86/dma-mapping.h
+++ b/include/asm-x86/dma-mapping.h
@@ -246,6 +246,8 @@ dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
 	struct dma_mapping_ops *ops = get_dma_ops(dev);
 	void *memory;
 
+	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32);
+
 	if (dma_alloc_from_coherent(dev, size, dma_handle, &memory))
 		return memory;
 
-- 
1.5.4.2

--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: [PATCH] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
Date: Friday, September 5, 2008 - 1:58 am

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-nommu.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index 526b2db..3495f88 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -102,6 +102,9 @@ nommu_alloc_coherent(struct device *hwdev, size_t size,
 	if (dma_mask == DMA_24BIT_MASK)
 		gfp |= GFP_DMA;
 again:
+	if (gfp & GFP_DMA)
+		gfp |= __GFP_NORETRY;
+
 	page = alloc_pages_node(node, gfp, get_order(size));
 	if (!page)
 		return NULL;
-- 
1.5.4.2

--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: [PATCH] x86: gart alloc_coherent doesn't need to check NULL device argument
Date: Friday, September 5, 2008 - 1:58 am

asm/dma-mapping.h guarantees that gart alloc_coherent doesn't get NULL
device argument.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-gart_64.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index 4d08649..0b99d4a 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -506,9 +506,6 @@ gart_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_addr,
 
 	align_mask = (1UL << get_order(size)) - 1;
 
-	if (!dev)
-		dev = &x86_dma_fallback_dev;
-
 	*dma_addr = dma_map_area(dev, __pa(vaddr), size, DMA_BIDIRECTIONAL,
 				 align_mask);
 	flush_gart();
-- 
1.5.4.2

--


[ message continues ]
[view original message]
From: Joerg Roedel
Subject: Re: [PATCH] x86: gart alloc_coherent doesn't need to check NULL device argument
Date: Friday, September 5, 2008 - 3:41 am

True, thanks.



-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

--


[ message continues ]
[view original message]
From: Joerg Roedel
Subject: Re: [PATCH] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
Date: Friday, September 5, 2008 - 3:43 am

Huh? Why that? The __GFP_NORETRY is a hint from the caller to the page
allocator on how aggressive it should try to allocate memory. I don't
think the DMA code should touch those flags unless there is a very very

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: Re: [PATCH] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
Date: Saturday, September 6, 2008 - 5:18 am

On Fri, 5 Sep 2008 12:43:05 +0200

The current comment is reasonable for me:

/* Don't invoke OOM killer or retry in lower 16MB DMA zone */
if (gfp & __GFP_DMA)
     noretry = 1;
--


[ message continues ]
[view original message]
From: Joerg Roedel
Subject: Re: [PATCH] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
Date: Monday, September 8, 2008 - 4:41 am

Ok, so please add a comment to this so we know in the future why this is
there.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

--


[ message continues ]
[view original message]
From: Joerg Roedel
Subject: Re: [PATCH] x86: fix nommu_alloc_coherent allocation with NULL device argument
Date: Friday, September 5, 2008 - 3:40 am

I agree with this patch in principle. It fixes the problem. But I think
we should evaluate if we can change the dma_mask of the fallback_dev to
24 bit.


Please leave these spare lines where they are. They increase the

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

--


[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH] x86: fix nommu_alloc_coherent allocation with NULL device argument
Date: Friday, September 5, 2008 - 3:49 am

ok - i kept this commit for now in tip/x86/iommu:

# 551b454: x86: gart alloc_coherent doesn't need to check NULL device argument

and will let you guys sort out how to do the others.

	Ingo
--


[ message continues ]
[view original message]
From: Joerg Roedel
Subject: Re: [PATCH] x86: fix nommu_alloc_coherent allocation with NULL device argument
Date: Friday, September 5, 2008 - 4:56 am

Andi,

I guess you made the dma_mask of the fallback_dev 32 bit a long time
ago. Is there any reason it cannot be 24 bit?

Joerg


-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy

--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: Re: [PATCH] x86: fix nommu_alloc_coherent allocation with NULL device argument
Date: Saturday, September 6, 2008 - 5:18 am

On Fri, 5 Sep 2008 12:40:49 +0200

That's fine by me if someone who wrote the current code say that the
change is safe (though I think it's safe).
--


[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: Re: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Friday, September 5, 2008 - 2:11 am

On Fri,  5 Sep 2008 17:58:46 +0900

Oops, I messed up the subjects. They should have been:

[PATCH 1/3] x86: fix nommu_alloc_coherent allocation with NULL device argument
[PATCH 2/3] x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
[PATCH 3/3] x86: gart alloc_coherent doesn't need to check NULL device argument

--


[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Friday, September 5, 2008 - 3:45 am

I've applied them to tip/x86/iommu:

 52fceb1: x86: gart alloc_coherent doesn't need to check NULL device argument
 150ba17: x86: use __GFP_NORETRY in the case of GFP_DMA with pci-nommu
 3b3d509: x86: fix nommu_alloc_coherent allocation with NULL device argument


that's OK - i dont rely on the numbering when picking up patches and 
they get discarded by git-am for the commit log anyway.

The only real use for numbering is when there's some really large set of 
patches (dozens of them) where i'd like to make sure no mail got dropped 
or reordered before i do some more difficult merge or conflict 
resolution run.

	Ingo
--


[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Monday, September 8, 2008 - 10:35 am

btw., the build became very noisy - see the messages below.

	Ingo

---------->
init/initramfs.c:517: warning: 'clean_rootfs' defined but not used
In file included from include/linux/dma-mapping.h:53,
                 from include/linux/dmaengine.h:30,
                 from include/linux/skbuff.h:30,
                 from include/linux/netlink.h:156,
                 from include/linux/genetlink.h:5,
                 from include/net/genetlink.h:5,
                 from include/linux/taskstats_kern.h:13,
                 from init/main.c:48:
include/asm/dma-mapping.h: In function 'dma_alloc_coherent_gfp_flags':
include/asm/dma-mapping.h:258: warning: unused variable 'dma_mask'
In file included from include/linux/dma-mapping.h:53,
                 from arch/x86/kernel/pci-dma.c:2:
include/asm/dma-mapping.h: In function 'dma_alloc_coherent_gfp_flags':
include/asm/dma-mapping.h:258: warning: unused variable 'dma_mask'
In file included from include/linux/dma-mapping.h:53,
                 from include/asm-generic/pci-dma-compat.h:8,
                 from include/asm/pci.h:97,
                 from include/linux/pci.h:990,
                 from arch/x86/kernel/probe_roms_32.c:13:
include/asm/dma-mapping.h: In function 'dma_alloc_coherent_gfp_flags':
include/asm/dma-mapping.h:258: warning: unused variable 'dma_mask'
In file included from include/linux/dma-mapping.h:53,
                 from include/asm-generic/pci-dma-compat.h:8,
                 from include/asm/pci.h:97,
                 from include/linux/pci.h:990,
                 from arch/x86/kernel/pci-nommu.c:6:
include/asm/dma-mapping.h: In function 'dma_alloc_coherent_gfp_flags':
include/asm/dma-mapping.h:258: warning: unused variable 'dma_mask'
In file included from include/linux/dma-mapping.h:53,
                 from include/asm-generic/pci-dma-compat.h:8,
                 from include/asm/pci.h:97,
                 from include/linux/pci.h:990,
                 from ...
[ message continues ]
[view original message]
From: FUJITA Tomonori
Subject: Re: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Monday, September 8, 2008 - 11:06 am

On Mon, 8 Sep 2008 19:35:25 +0200

Very sorry, I should have compile this on X86_32...

Can you replace the fourth patch with this?

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH] x86: dma_alloc_coherent sets gfp flags properly

Non real IOMMU implemenations (which doesn't do virtual mappings,
e.g. swiotlb, pci-nommu, etc) need to use proper gfp flags and
dma_mask to allocate pages in their own dma_alloc_coherent()
(allocated page need to be suitable for device's coherent_dma_mask).

This patch makes dma_alloc_coherent do this job so that IOMMUs don't
need to take care of it any more.

Real IOMMU implemenataions can simply ignore the gfp flags.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 arch/x86/kernel/pci-nommu.c   |   19 ++-----------------
 include/asm-x86/dma-mapping.h |   32 ++++++++++++++++++++++++++++----
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kernel/pci-nommu.c b/arch/x86/kernel/pci-nommu.c
index ada1c87..8e398b5 100644
--- a/arch/x86/kernel/pci-nommu.c
+++ b/arch/x86/kernel/pci-nommu.c
@@ -80,26 +80,11 @@ nommu_alloc_coherent(struct device *hwdev, size_t size,
 	int node;
 	struct page *page;
 
-	gfp |= __GFP_ZERO;
-
-	dma_mask = hwdev->coherent_dma_mask;
-	if (!dma_mask)
-		dma_mask = *(hwdev->dma_mask);
+	dma_mask = dma_alloc_coherent_mask(hwdev, gfp);
 
-	if (dma_mask < DMA_24BIT_MASK)
-		return NULL;
+	gfp |= __GFP_ZERO;
 
 	node = dev_to_node(hwdev);
-
-#ifdef CONFIG_X86_64
-	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA))
-		gfp |= GFP_DMA32;
-#endif
-
-	/* No alloc-free penalty for ISA devices */
-	if (dma_mask == DMA_24BIT_MASK)
-		gfp |= GFP_DMA;
-
 again:
 	page = alloc_pages_node(node, gfp, get_order(size));
 	if (!page)
diff --git a/include/asm-x86/dma-mapping.h b/include/asm-x86/dma-mapping.h
index 9d6dcf4..8bb3108 100644
--- a/include/asm-x86/dma-mapping.h
+++ b/include/asm-x86/dma-mapping.h
@@ -241,6 +241,29 @@ static inline int ...
[ message continues ]
[view original message]
From: Ingo Molnar
Subject: Re: [PATCH 0/3] fix alloc_coherent allocation issues (tip/x86/iommu)
Date: Monday, September 8, 2008 - 11:15 am

some other commits came inbetween already - i applied the delta below.

	Ingo

--------------->
From 53f79ced9d2c42a52290b460b88a4720a481a3ed Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Mon, 8 Sep 2008 20:13:06 +0200
Subject: [PATCH] x86: fix alloc_coherent allocation issues (tip/x86/iommu), warning fixes

fix warnings.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/dma-mapping.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/asm-x86/dma-mapping.h b/include/asm-x86/dma-mapping.h
index 0cc022b..5607532 100644
--- a/include/asm-x86/dma-mapping.h
+++ b/include/asm-x86/dma-mapping.h
@@ -253,9 +253,9 @@ static inline unsigned long dma_alloc_coherent_mask(struct device *dev,
 
 static inline gfp_t dma_alloc_coherent_gfp_flags(struct device *dev, gfp_t gfp)
 {
+#ifdef CONFIG_X86_64
 	unsigned long dma_mask = dma_alloc_coherent_mask(dev, gfp);
 
-#ifdef CONFIG_X86_64
 	if (dma_mask <= DMA_32BIT_MASK && !(gfp & GFP_DMA))
 		gfp |= GFP_DMA32;
 #endif
--


[ message continues ]
Previous thread: none

Next thread: linux-next: Tree for September 5 by Stephen Rothwell on Friday, September 5, 2008 - 2:09 am. (4 messages)