Skip to contentSkip to conversation

Add API for DMA memory reservation for devices

PATCH New Guennadi Liakhovetski 7 participants 18 messages Aug 19, 2010 – Sep 16, 2010

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Add API for DMA memory reservation for devices

This patch series attempts to solve the problem discussed here:
http://marc.info/?t=128128236400002&r=1&w=2. It is a regression in 2.6.36-rc1, so, we shall try to fix it ASAP. The series contains a couple of clean ups, the actual implementation and an example of using this new API. Tested on SuperH. Comments welcome.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

#1 lore raw

PATCH 1/5 DMA: Remove unneeded "extern" from function declarations

---
 include/asm-generic/dma-coherent.h |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
include/asm-generic/dma-coherent.h +3 -6
index 85a3ffa..a92bc09 100644
@@ -14,15 +14,12 @@ int dma_release_from_coherent(struct device *dev, int order, void *vaddr);
  * Standard interface
  */
 #define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY
-extern int
-dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
+int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
 			    dma_addr_t device_addr, size_t size, int flags);
 
-extern void
-dma_release_declared_memory(struct device *dev);
+void dma_release_declared_memory(struct device *dev);
 
-extern void *
-dma_mark_declared_memory_occupied(struct device *dev,
+void *dma_mark_declared_memory_occupied(struct device *dev,
 				  dma_addr_t device_addr, size_t size);
 #else
 #define dma_alloc_from_coherent(dev, size, handle, ret) (0)
Signature · 1 line
1.7.2
#2 lore raw

PATCH 2/5 DMA: dma_declare_coherent_memory() should return an error if unsupported

In reply to Guennadi Liakhovetski

On platforms, that do not define ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY, which are currently all platforms, that do not select CONFIG_HAVE_GENERIC_DMA_COHERENT, dma_declare_coherent_memory() is a NOP, which means, device internal memory cannot really be used. Therefore dma_declare_coherent_memory() has to return an error on such platforms.

---
 include/linux/dma-mapping.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/dma-mapping.h +1 -1
index ce29b81..77ce97f 100644
@@ -163,7 +163,7 @@ static inline int
 dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
 			    dma_addr_t device_addr, size_t size, int flags)
 {
-	return 0;
+	return -EINVAL;
 }
 
 static inline void
Signature · 1 line
1.7.2
#3 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

In reply to Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

---
 drivers/base/dma-coherent.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)
drivers/base/dma-coherent.c +3 -8
index f369e27..8efdfd4 100644
@@ -11,7 +11,7 @@ struct dma_coherent_mem {
 	dma_addr_t	device_base;
 	int		size;
 	int		flags;
-	unsigned long	*bitmap;
+	unsigned long	bitmap[0];
 };
 
 int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
@@ -34,12 +34,10 @@ int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
 	if (!mem_base)
 		goto out;
 
-	dev->dma_mem = kzalloc(sizeof(struct dma_coherent_mem), GFP_KERNEL);
+	dev->dma_mem = kzalloc(sizeof(struct dma_coherent_mem) + bitmap_size,
+			       GFP_KERNEL);
 	if (!dev->dma_mem)
 		goto out;
-	dev->dma_mem->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
-	if (!dev->dma_mem->bitmap)
-		goto free1_out;
 
 	dev->dma_mem->virt_base = mem_base;
 	dev->dma_mem->device_base = device_addr;
@@ -51,8 +49,6 @@ int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
 
 	return DMA_MEMORY_IO;
 
- free1_out:
-	kfree(dev->dma_mem);
  out:
 	if (mem_base)
 		iounmap(mem_base);
@@ -68,7 +64,6 @@ void dma_release_declared_memory(struct device *dev)
 		return;
 	dev->dma_mem = NULL;
 	iounmap(mem->virt_base);
-	kfree(mem->bitmap);
 	kfree(mem);
 }
 EXPORT_SYMBOL(dma_release_declared_memory);
Signature · 1 line
1.7.2
#4 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 2 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

You describe what you did, but _why_ are you doing it?

thanks,

greg k-h

#5 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 5 lines · Greg KH
Quoted 2 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

You describe what you did, but _why_ are you doing it?

Well, to me the subject "_reduce_ the number..." is already a reason enough - the fewer kmalloc's of 20 bytes - the better, I think. But if you like, I can update the description too.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

#6 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 11 lines · Guennadi Liakhovetski
Quoted 5 lines · Greg KH
Quoted 2 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

You describe what you did, but _why_ are you doing it?

Well, to me the subject "_reduce_ the number..." is already a reason enough - the fewer kmalloc's of 20 bytes - the better, I think. But if you like, I can update the description too.

Yes, as I missed that, others might as well :)

thanks,

greg k-h

#7 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 7 lines · Greg KH
Quoted 6 lines · Guennadi Liakhovetski
Quoted 4 lines · Greg KH
Quoted 2 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

You describe what you did, but _why_ are you doing it?

Well, to me the subject "_reduce_ the number..." is already a reason enough - the fewer kmalloc's of 20 bytes - the better, I think. But if you like, I can update the description too.

Yes, as I missed that, others might as well :)

Indeed. When this is reposted, it should also include an update for Documentation/DMA-API.txt. I'm simply not going to take any DMA API patches if they don't include the documentation update. DMA-API.txt is one of the few useful parts of Documentation/, we should really strive to keep it that way.

It would also be nice if on the next iteration DMA API people were Cc'ed, this would be at least:

FUJITA Tomonori <[email protected]> James Bottomley <[email protected]>

#8 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 10 lines · Paul Mundt
Quoted 9 lines · Greg KH
Quoted 8 lines · Guennadi Liakhovetski
Quoted 5 lines · Greg KH
Quoted 2 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

You describe what you did, but _why_ are you doing it?

Quoted 7 lines
Quoted 6 lines
Quoted 3 lines

Well, to me the subject "_reduce_ the number..." is already a reason enough - the fewer kmalloc's of 20 bytes - the better, I think. But if you like, I can update the description too.

Yes, as I missed that, others might as well :)

Indeed. When this is reposted,

Ok, can do that.

Quoted 5 lines

it should also include an update for Documentation/DMA-API.txt. I'm simply not going to take any DMA API patches if they don't include the documentation update. DMA-API.txt is one of the few useful parts of Documentation/, we should really strive to keep it that way.

I believe, this your comment refers to another related patch of mine:

[RFC][PATCH] add dma_reserve_coherent_memory()/dma_free_reserved_memory() API

Sure, updating the documentation would be the right thing to do here, but FUJITA Tomonori is nacking this patch for 2.6.36, so, we will either have to live with a few broken platforms and drivers until 2.6.37, or we'll have to find another solution, or that NAK has to be reconsidered

Quoted 5 lines

It would also be nice if on the next iteration DMA API people were Cc'ed, this would be at least:

FUJITA Tomonori <[email protected]> James Bottomley <[email protected]>

Right, sorry, added to the CC now, although, this is not the right thread to discuss the proposed new API.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

#9 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 6 lines · On Wed, 15 Sep 2010 09:23:54 +0200 (CEST) Guennadi Liakhovetski…
Quoted 5 lines

it should also include an update for Documentation/DMA-API.txt. I'm simply not going to take any DMA API patches if they don't include the documentation update. DMA-API.txt is one of the few useful parts of Documentation/, we should really strive to keep it that way.

Quoted 6 lines

I believe, this your comment refers to another related patch of mine:

[RFC][PATCH] add dma_reserve_coherent_memory()/dma_free_reserved_memory() API

Sure, updating the documentation would be the right thing to do here, but FUJITA Tomonori is nacking this patch for 2.6.36, so, we will either have

I already explained why the above API is wrong.

Quoted 2 lines

to live with a few broken platforms and drivers until 2.6.37, or we'll have to find another solution, or that NAK has to be reconsidered

Needs to revert the patch that causes the regression. The patch does the right thing but also breaks some working platforms. You need to fix "working" platforms that does wrong things before the patch.

Quoted 8 lines
Quoted 5 lines

It would also be nice if on the next iteration DMA API people were Cc'ed, this would be at least:

FUJITA Tomonori <[email protected]> James Bottomley <[email protected]>

Right, sorry, added to the CC now, although, this is not the right thread to discuss the proposed new API.

Please send to linux-kernel and linux-arch too.

#10 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 6 lines · FUJITA Tomonori
Quoted 2 lines

to live with a few broken platforms and drivers until 2.6.37, or we'll have to find another solution, or that NAK has to be reconsidered

Needs to revert the patch that causes the regression. The patch does the right thing but also breaks some working platforms. You need to fix "working" platforms that does wrong things before the patch.

Not going to happen.

#11 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

Quoted 9 lines · On Thu, 16 Sep 2010 08:04:42 +0100 Russell King - ARM Linux…
Quoted 6 lines · FUJITA Tomonori
Quoted 2 lines

to live with a few broken platforms and drivers until 2.6.37, or we'll have to find another solution, or that NAK has to be reconsidered

Needs to revert the patch that causes the regression. The patch does the right thing but also breaks some working platforms. You need to fix "working" platforms that does wrong things before the patch.

Not going to happen.

Then, as Paul said, we live with the breakage for 2.6.36; systems that rely on wrong behaviors could be broken.

#12 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

In reply to Guennadi Liakhovetski
Quoted 9 lines · Guennadi Liakhovetski
Quoted 5 lines · Paul Mundt

it should also include an update for Documentation/DMA-API.txt. I'm simply not going to take any DMA API patches if they don't include the documentation update. DMA-API.txt is one of the few useful parts of Documentation/, we should really strive to keep it that way.

I believe, this your comment refers to another related patch of mine:

Yes, sorry about that, this is what I get for having two mutt sessions open at the same time. I ended up reading one patch and replying to the other.

Quoted 5 lines

Sure, updating the documentation would be the right thing to do here, but FUJITA Tomonori is nacking this patch for 2.6.36, so, we will either have to live with a few broken platforms and drivers until 2.6.37, or we'll have to find another solution, or that NAK has to be reconsidered

I suppose we can live with the breakage for 2.6.36 given that it's just enforcing behaviour that shouldn't have been relied upon in the first place. At least there is the opportunity to get things fixed up properly for 2.6.37, and people that invariably care about it on 2.6.36 will just end up backporting anyways.

#13 lore raw

PATCH 3/5 DMA: reduce the number of memory allocations

In reply to Guennadi Liakhovetski
Quoted 13 lines · Guennadi Liakhovetski

Allocate the bitmap in dma_declare_coherent_memory() together with the struct dma_coherent_mem object.

---
 drivers/base/dma-coherent.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)
drivers/base/dma-coherent.c +0 -0
index f369e27..8efdfd4 100644

drivers/base/dma-coherent.c struct dma_coherent_mem { · line 15

Quoted 6 lines
drivers/base/dma-coherent.c +1 -1
@@ -11,7 +11,7 @@ struct dma_coherent_mem {
 	dma_addr_t	device_base;
 	int		size;
 	int		flags;
-	unsigned long	*bitmap;
+	unsigned long	bitmap[0];
 };

Wouldn't "bitmap[]" be better? AFAIR "bitmap[0]" is really a GCC extension and C99 has introduced "bitmap[]" for exactly the above purpose.

Signature · 4 lines
Best regards,                                        _     _
| Humble Liege of Serenely Enlightened Majesty of  o' \,=./ `o
| Computer Science,  Micha? "mina86" Nazarewicz       (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
#14 lore raw

PATCH 4/5 DMA: Add functions to preallocate DMA memory dor devices

In reply to Guennadi Liakhovetski

Currently dma_declare_coherent_memory() and dma_release_declared_memory() are provided to assign DMA memory to a device and to release it. These functions can be use with device local memory, like on-chip SRAM. However, they are unsuitable for assigning of generic system RAM to devices, because such system memory should not be ioremapped, and ioremap() is used internally in the former of these functions to map the memory.

This patch solves this problem by adding two more functions:
dma_preallocate_coherent_memory() and dma_release_preallocated_memory(), which allocate DMA coherent memory and assign it to the device, and release it back respectively.

---
 drivers/base/dma-coherent.c        |   50 ++++++++++++++++++++++++++++++++++++
 include/asm-generic/dma-coherent.h |    2 +
 include/linux/dma-mapping.h        |   12 ++++++++
 3 files changed, 64 insertions(+), 0 deletions(-)
drivers/base/dma-coherent.c +50 -0
index 8efdfd4..9eb2856 100644
@@ -170,3 +170,53 @@ int dma_release_from_coherent(struct device *dev, int order, void *vaddr)
 	return 0;
 }
 EXPORT_SYMBOL(dma_release_from_coherent);
+
+int dma_preallocate_coherent_memory(struct device *dev, size_t size, gfp_t gfp,
+				    int flags)
+{
+	int pages = size >> PAGE_SHIFT;
+	int bitmap_size = BITS_TO_LONGS(pages) * sizeof(long);
+	dma_addr_t dma_handle;
+	void *buf;
+
+	if (!(flags & (DMA_MEMORY_MAP | DMA_MEMORY_IO)) || !dev)
+		return -EINVAL;
+
+	if (!size)
+		return 0;
+
+	buf = dma_alloc_coherent(NULL, size, &dma_handle, gfp);
+	if (!buf)
+		return -ENOMEM;
+
+	dev->dma_mem = kzalloc(sizeof(struct dma_coherent_mem) + bitmap_size, gfp);
+	if (!dev->dma_mem)
+		goto ealloc;
+
+	dev->dma_mem->virt_base = buf;
+	dev->dma_mem->device_base = dma_handle;
+	dev->dma_mem->size = pages;
+	dev->dma_mem->flags = flags;
+
+	return 0;
+
+ealloc:
+	dma_free_coherent(dev, size, buf, dma_handle);
+
+	return -ENOMEM;
+}
+EXPORT_SYMBOL(dma_preallocate_coherent_memory);
+
+void dma_release_preallocated_memory(struct device *dev)
+{
+	struct dma_coherent_mem *mem = dev->dma_mem;
+
+	if (!mem)
+		return;
+
+	dev->dma_mem = NULL;
+	dma_free_coherent(dev, mem->size << PAGE_SHIFT, mem->virt_base,
+			  mem->device_base);
+	kfree(mem);
+}
+EXPORT_SYMBOL(dma_release_preallocated_memory);
include/asm-generic/dma-coherent.h +2 -0
index a92bc09..08a66a1 100644
@@ -21,6 +21,8 @@ void dma_release_declared_memory(struct device *dev);
 
 void *dma_mark_declared_memory_occupied(struct device *dev,
 				  dma_addr_t device_addr, size_t size);
+int dma_preallocate_coherent_memory(struct device *dev, size_t size, gfp_t gfp, int flags);
+void dma_release_preallocated_memory(struct device *dev);
 #else
 #define dma_alloc_from_coherent(dev, size, handle, ret) (0)
 #define dma_release_from_coherent(dev, order, vaddr) (0)
include/linux/dma-mapping.h +12 -0
index 77ce97f..0912abf 100644
@@ -177,6 +177,18 @@ dma_mark_declared_memory_occupied(struct device *dev,
 {
 	return ERR_PTR(-EBUSY);
 }
+
+static inline int
+dma_preallocate_coherent_memory(struct device *dev, size_t size, gfp_t gfp, int flags)
+{
+	return -EINVAL;
+}
+
+static inline void
+dma_release_preallocated_memory(struct device *dev)
+{
+}
+
 #endif
 
 /*
Signature · 1 line
1.7.2
#15 lore raw

PATCH 4/5 DMA: Add functions to preallocate DMA memory dor devices

Quoted 11 lines · Guennadi Liakhovetski

Currently dma_declare_coherent_memory() and dma_release_declared_memory() are provided to assign DMA memory to a device and to release it. These functions can be use with device local memory, like on-chip SRAM. However, they are unsuitable for assigning of generic system RAM to devices, because such system memory should not be ioremapped, and ioremap() is used internally in the former of these functions to map the memory.

This patch solves this problem by adding two more functions:
dma_preallocate_coherent_memory() and dma_release_preallocated_memory(), which allocate DMA coherent memory and assign it to the device, and release it back respectively.

This looks like a much saner interface for pre-allocating DMA memory from system memory, rather than trying to use dma_declare_coherent_memory().

#16 lore raw

PATCH 4/5 DMA: Add functions to preallocate DMA memory dor devices

Hello,

Quoted 13 lines · Russell King - ARM Linux
Quoted 11 lines · Guennadi Liakhovetski

Currently dma_declare_coherent_memory() and dma_release_declared_memory() are provided to assign DMA memory to a device and to release it. These functions can be use with device local memory, like on-chip SRAM. However, they are unsuitable for assigning of generic system RAM to devices, because such system memory should not be ioremapped, and ioremap() is used internally in the former of these functions to map the memory.

This patch solves this problem by adding two more functions:
dma_preallocate_coherent_memory() and dma_release_preallocated_memory(), which allocate DMA coherent memory and assign it to the device, and release it back respectively.

Quoted 4 lines

This looks like a much saner interface for pre-allocating DMA memory from system memory, rather than trying to use dma_declare_coherent_memory().

If this series isn't acceptable for the DMA people, I suggest to add it locally for ARM if no other solution is found before .36 is released.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
#17 lore raw

PATCH 5/5 SH: use dma_preallocate_coherent_memory() for platform device memory

In reply to Guennadi Liakhovetski

platform_resource_setup_memory() is used on SuperH to allocate large chunks of memory at system initialisation time for platform devices. Many of those devices are then handled by the UIO subsystem, and only one of them - the CEU has an in-kernel driver. This patch switches platform_resource_setup_memory() to use dma_preallocate_coherent_memory() and removes now unneeded call to dma_declare_coherent_memory() from the CEU driver.

---
 arch/sh/mm/consistent.c                    |   19 +++++++++----------
 drivers/media/video/sh_mobile_ceu_camera.c |   19 +------------------
 2 files changed, 10 insertions(+), 28 deletions(-)
arch/sh/mm/consistent.c +9 -10
index c86a085..c617b99 100644
@@ -127,8 +127,7 @@ int __init platform_resource_setup_memory(struct platform_device *pdev,
 					  char *name, unsigned long memsize)
 {
 	struct resource *r;
-	dma_addr_t dma_handle;
-	void *buf;
+	int ret;
 
 	r = pdev->resource + pdev->num_resources - 1;
 	if (r->flags) {
@@ -141,17 +140,17 @@ int __init platform_resource_setup_memory(struct platform_device *pdev,
 	if (!memsize)
 		return 0;
 
-	buf = dma_alloc_coherent(NULL, memsize, &dma_handle, GFP_KERNEL);
-	if (!buf) {
-		pr_warning("%s: unable to allocate memory\n", name);
-		return -ENOMEM;
+	ret = dma_preallocate_coherent_memory(&pdev->dev, memsize, GFP_KERNEL,
+					      DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);
+	if (ret < 0) {
+		pr_warning("%s: unable to allocate memory: %d\n", name, ret);
+		return ret;
 	}
 
-	memset(buf, 0, memsize);
-
 	r->flags = IORESOURCE_MEM;
-	r->start = dma_handle;
-	r->end = r->start + memsize - 1;
+	/* Only the size is needed */
+	r->start = 0;
+	r->end = memsize - 1;
 	r->name = name;
 	return 0;
 }
drivers/media/video/sh_mobile_ceu_camera.c +1 -18
index bfddad8..325671e 100644
@@ -1951,20 +1951,8 @@ static int __devinit sh_mobile_ceu_probe(struct platform_device *pdev)
 	pcdev->video_limit = 0; /* only enabled if second resource exists */
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (res) {
-		err = dma_declare_coherent_memory(&pdev->dev, res->start,
-						  res->start,
-						  resource_size(res),
-						  DMA_MEMORY_MAP |
-						  DMA_MEMORY_EXCLUSIVE);
-		if (!err) {
-			dev_err(&pdev->dev, "Unable to declare CEU memory.\n");
-			err = -ENXIO;
-			goto exit_iounmap;
-		}
-
+	if (res)
 		pcdev->video_limit = resource_size(res);
-	}
 
 	/* request irq */
 	err = request_irq(pcdev->irq, sh_mobile_ceu_irq, IRQF_DISABLED,
@@ -2036,9 +2024,6 @@ exit_free_clk:
 	pm_runtime_disable(&pdev->dev);
 	free_irq(pcdev->irq, pcdev);
 exit_release_mem:
-	if (platform_get_resource(pdev, IORESOURCE_MEM, 1))
-		dma_release_declared_memory(&pdev->dev);
-exit_iounmap:
 	iounmap(base);
 exit_kfree:
 	kfree(pcdev);
@@ -2056,8 +2041,6 @@ static int __devexit sh_mobile_ceu_remove(struct platform_device *pdev)
 	soc_camera_host_unregister(soc_host);
 	pm_runtime_disable(&pdev->dev);
 	free_irq(pcdev->irq, pcdev);
-	if (platform_get_resource(pdev, IORESOURCE_MEM, 1))
-		dma_release_declared_memory(&pdev->dev);
 	iounmap(pcdev->base);
 	if (csi2 && csi2->driver)
 		module_put(csi2->driver->owner);
Signature · 1 line
1.7.2