[PATCH 0/3] Add code for setting coherent pool size

[PATCH 0/3] Add code for setting coherent pool size

[Marek Szyprowski]

Hi!

Aaro Koskinen and Josh Coombs reported that commit e9da6e9905e639 ("ARM:
dma-mapping: remove custom consistent dma region") introduced a
regresion. It turned out that the default 256KiB for atomic coherent
pool might not be enough. After that patch, some Kirkwood systems run
out of atomic coherent memory and fail without any meanfull message.

This patch series is an attempt to fix those issues by adding function
for setting coherent pool size from platform initialization code and
increasing the size of the pool for Kirkwood systems.

Best regards
Marek Szyprowski
Samsung Poland R&D Center

Patch summary:

Marek Szyprowski (3):
  ARM: DMA-Mapping: add function for setting coherent pool size from
    platform code
  ARM: DMA-Mapping: print warning when atomic coherent allocation fails
  ARM: Kirkwood: increase atomic coherent pool size

 arch/arm/include/asm/dma-mapping.h |    7 +++++++
 arch/arm/mach-kirkwood/common.c    |    7 +++++++
 arch/arm/mm/dma-mapping.c          |   22 +++++++++++++++++++++-
 3 files changed, 35 insertions(+), 1 deletions(-)

-- 
1.7.1.569.g6f426

[PATCH 1/3] ARM: DMA-Mapping: add function for setting coherent pool size from platform code

[Marek Szyprowski]

Some platforms might require to increase atomic coherent pool to make
sure that their device will be able to allocate all their buffers from
atomic context. This function can be also used to decrease atomic
coherent pool size if coherent allocations are not used for the given
sub-platform.

Suggested-by: Josh Coombs <josh.coombs@gmail.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/include/asm/dma-mapping.h |    7 +++++++
 arch/arm/mm/dma-mapping.c          |   19 ++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index 2ae842d..a53940d 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -203,6 +203,13 @@ static inline void dma_free_writecombine(struct device *dev, size_t size,
 }
 
 /*
+ * This can be called during early boot to increase the size of the atomic
+ * coherent DMA pool above the default value of 256KiB. It must be called 
+ * before postcore_initcall.
+ */
+extern void __init init_dma_coherent_pool_size(unsigned long size);
+
+/*
  * This can be called during boot to increase the size of the consistent
  * DMA region above it's default value of 2MB. It must be called before the
  * memory allocator is initialised, i.e. before any core_initcall.
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 4e7d118..d1cc9c1 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -267,6 +267,8 @@ static void __dma_free_remap(void *cpu_addr, size_t size)
 	vunmap(cpu_addr);
 }
 
+#define DEFAULT_DMA_COHERENT_POOL_SIZE	SZ_256K
+
 struct dma_pool {
 	size_t size;
 	spinlock_t lock;
@@ -277,7 +279,7 @@ struct dma_pool {
 };
 
 static struct dma_pool atomic_pool = {
-	.size = SZ_256K,
+	.size = DEFAULT_DMA_COHERENT_POOL_SIZE,
 };
 
 static int __init early_coherent_pool(char *p)
@@ -287,6 +289,21 @@ static int __init early_coherent_pool(char *p)
 }
 early_param("coherent_pool", early_coherent_pool);
 
+void __init init_dma_coherent_pool_size(unsigned long size)
+{
+	/*
+	 * Catch any attempt to set the pool size too late.
+	 */
+	BUG_ON(atomic_pool.vaddr);
+
+	/*
+	 * Set architecture specific coherent pool size only if
+	 * it has not been changed by kernel command line parameter.
+	 */
+	if (atomic_pool.size == DEFAULT_DMA_COHERENT_POOL_SIZE)
+		atomic_pool.size = size;
+}
+
 /*
  * Initialise the coherent pool for atomic allocations.
  */
-- 
1.7.1.569.g6f426

[PATCH 2/3] ARM: DMA-Mapping: print warning when atomic coherent allocation fails

[Marek Szyprowski]

Print a loud warning when system runs out of memory from atomic coherent
pool to let users notice the potential problem.

Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index d1cc9c1..25963ea 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -461,6 +461,9 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
 		bitmap_set(pool->bitmap, pageno, count);
 		ptr = pool->vaddr + PAGE_SIZE * pageno;
 		*ret_page = pool->page + pageno;
+	} else {
+		pr_err("Atomic coherent pool too small!\n"
+		       "Please increase it with coherent_pool= kernel parameter!\n");
 	}
 	spin_unlock_irqrestore(&pool->lock, flags);
 
-- 
1.7.1.569.g6f426

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Marek Szyprowski]

The default 256 KiB coherent pool may be too small for some of the Kirkwood
devices, so increase it to make sure that devices will be able to allocate
their buffers with GFP_ATOMIC flag.

Suggested-by: Josh Coombs <josh.coombs@gmail.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mach-kirkwood/common.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
index c4b64ad..d748f50 100644
--- a/arch/arm/mach-kirkwood/common.c
+++ b/arch/arm/mach-kirkwood/common.c
@@ -517,6 +517,13 @@ void __init kirkwood_wdt_init(void)
 void __init kirkwood_init_early(void)
 {
 	orion_time_set_base(TIMER_VIRT_BASE);
+
+	/*
+	 * Some Kirkwood devices allocate their coherent buffers from atomic
+	 * context. Increase size of atomic coherent pool to make sure such
+	 * the allocations won't fail.
+	 */
+	init_dma_coherent_pool_size(SZ_1M);
 }
 
 int kirkwood_tclk;
-- 
1.7.1.569.g6f426

[PATCH 2/3] ARM: DMA-Mapping: print warning when atomic coherent allocation fails

[Aaro Koskinen]

Hi,

On Mon, Aug 20, 2012 at 12:47:27PM +0200, Marek Szyprowski wrote:
> @@ -461,6 +461,9 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
>  		bitmap_set(pool->bitmap, pageno, count);
>  		ptr = pool->vaddr + PAGE_SIZE * pageno;
>  		*ret_page = pool->page + pageno;
> +	} else {
> +		pr_err("Atomic coherent pool too small!\n"
> +		       "Please increase it with coherent_pool= kernel parameter!\n");

This should be rate limited, perhaps even printed only once.

A.

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Aaro Koskinen]

On Mon, Aug 20, 2012 at 12:47:28PM +0200, Marek Szyprowski wrote:
> The default 256 KiB coherent pool may be too small for some of the Kirkwood
> devices, so increase it to make sure that devices will be able to allocate
> their buffers with GFP_ATOMIC flag.
[...]
> +	/*
> +	 * Some Kirkwood devices allocate their coherent buffers from atomic
> +	 * context. Increase size of atomic coherent pool to make sure such
> +	 * the allocations won't fail.
> +	 */
> +	init_dma_coherent_pool_size(SZ_1M);

Not sure if it's a valid use case, but what if some user wants to drop
e.g. SATA driver and and free up some memory. Would a smaller coherent
pool kernel parameter override this code in that case?

A.

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Marek Szyprowski]

Hello,

On Tuesday, August 21, 2012 1:14 AM Aaro Koskinen wrote:

> On Mon, Aug 20, 2012 at 12:47:28PM +0200, Marek Szyprowski wrote:
> > The default 256 KiB coherent pool may be too small for some of the Kirkwood
> > devices, so increase it to make sure that devices will be able to allocate
> > their buffers with GFP_ATOMIC flag.
> [...]
> > +	/*
> > +	 * Some Kirkwood devices allocate their coherent buffers from atomic
> > +	 * context. Increase size of atomic coherent pool to make sure such
> > +	 * the allocations won't fail.
> > +	 */
> > +	init_dma_coherent_pool_size(SZ_1M);
> 
> Not sure if it's a valid use case, but what if some user wants to drop
> e.g. SATA driver and and free up some memory. Would a smaller coherent
> pool kernel parameter override this code in that case?

Kernel command line parameter always overrides the value set by platform code
(or the default 256KiB).

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 2/3] ARM: DMA-Mapping: print warning when atomic coherent allocation fails

[Marek Szyprowski]

Hello,

On Tuesday, August 21, 2012 1:01 AM Aaro Koskinen wrote:

> On Mon, Aug 20, 2012 at 12:47:27PM +0200, Marek Szyprowski wrote:
> > @@ -461,6 +461,9 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
> >  		bitmap_set(pool->bitmap, pageno, count);
> >  		ptr = pool->vaddr + PAGE_SIZE * pageno;
> >  		*ret_page = pool->page + pageno;
> > +	} else {
> > +		pr_err("Atomic coherent pool too small!\n"
> > +		       "Please increase it with coherent_pool= kernel parameter!\n");
> 
> This should be rate limited, perhaps even printed only once.

Ok, I will change it to pr_err_once().

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[Linaro-mm-sig] [PATCH 2/3] ARM: DMA-Mapping: print warning when atomic coherent allocation fails

[Hiroshi Doyu]

On Mon, 20 Aug 2012 12:47:27 +0200
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> Print a loud warning when system runs out of memory from atomic coherent
> pool to let users notice the potential problem.
> 
> Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mm/dma-mapping.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index d1cc9c1..25963ea 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -461,6 +461,9 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
>  		bitmap_set(pool->bitmap, pageno, count);
>  		ptr = pool->vaddr + PAGE_SIZE * pageno;
>  		*ret_page = pool->page + pageno;
> +	} else {
> +		pr_err("Atomic coherent pool too small!\n"
> +		       "Please increase it with  kernel parameter!\n");

It might be a little bit nicer if the current pool size is printed in
the above too. That could be the hint for how much to specify in
"coherent_pool=".

[PATCH v2] ARM: DMA-Mapping: print warning when atomic coherent allocation fails

[Marek Szyprowski]

Print a loud warning when system runs out of memory from atomic DMA
coherent pool to let users notice the potential problem.

Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index d1cc9c1..acced93 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -461,6 +461,10 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page)
 		bitmap_set(pool->bitmap, pageno, count);
 		ptr = pool->vaddr + PAGE_SIZE * pageno;
 		*ret_page = pool->page + pageno;
+	} else {
+		pr_err_once("ERROR: %u KiB atomic DMA coherent pool is too small!\n"
+			    "Please increase it with coherent_pool= kernel parameter!\n",
+			    (unsigned)pool->size / 1024);
 	}
 	spin_unlock_irqrestore(&pool->lock, flags);
 
-- 
1.7.1.569.g6f426

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Jason Cooper]

On Mon, Aug 20, 2012 at 12:47:28PM +0200, Marek Szyprowski wrote:
> The default 256 KiB coherent pool may be too small for some of the Kirkwood
> devices, so increase it to make sure that devices will be able to allocate
> their buffers with GFP_ATOMIC flag.
> 
> Suggested-by: Josh Coombs <josh.coombs@gmail.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Acked-by: Jason Cooper <jason@lakedaemon.net>

I assume this will go through the dma tree.  Or, if they want to Ack it,
I'll take it through mine since I have boards depending on this.

thx,

Jason.

> ---
>  arch/arm/mach-kirkwood/common.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c
> index c4b64ad..d748f50 100644
> --- a/arch/arm/mach-kirkwood/common.c
> +++ b/arch/arm/mach-kirkwood/common.c
> @@ -517,6 +517,13 @@ void __init kirkwood_wdt_init(void)
>  void __init kirkwood_init_early(void)
>  {
>  	orion_time_set_base(TIMER_VIRT_BASE);
> +
> +	/*
> +	 * Some Kirkwood devices allocate their coherent buffers from atomic
> +	 * context. Increase size of atomic coherent pool to make sure such
> +	 * the allocations won't fail.
> +	 */
> +	init_dma_coherent_pool_size(SZ_1M);
>  }
>  
>  int kirkwood_tclk;
> -- 
> 1.7.1.569.g6f426
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Marek Szyprowski]

Hello Jason,

On 8/28/2012 8:25 PM, Jason Cooper wrote:
> On Mon, Aug 20, 2012 at 12:47:28PM +0200, Marek Szyprowski wrote:
>> The default 256 KiB coherent pool may be too small for some of the Kirkwood
>> devices, so increase it to make sure that devices will be able to allocate
>> their buffers with GFP_ATOMIC flag.
>>
>> Suggested-by: Josh Coombs <josh.coombs@gmail.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
>
> I assume this will go through the dma tree.  Or, if they want to Ack it,
> I'll take it through mine since I have boards depending on this.

Thanks for the ack, I would like to push it via my dma-mapping tree
together with the patches which add init_dma_coherent_pool_size()
function. I've already added it to fixes-for-3.6 branch.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH 3/3] ARM: Kirkwood: increase atomic coherent pool size

[Jason Cooper]

On Tue, Aug 28, 2012 at 08:37:57PM +0200, Marek Szyprowski wrote:
> Hello Jason,
> 
> On 8/28/2012 8:25 PM, Jason Cooper wrote:
> >On Mon, Aug 20, 2012 at 12:47:28PM +0200, Marek Szyprowski wrote:
> >>The default 256 KiB coherent pool may be too small for some of the Kirkwood
> >>devices, so increase it to make sure that devices will be able to allocate
> >>their buffers with GFP_ATOMIC flag.
> >>
> >>Suggested-by: Josh Coombs <josh.coombs@gmail.com>
> >>Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >
> >Acked-by: Jason Cooper <jason@lakedaemon.net>
> >
> >I assume this will go through the dma tree.  Or, if they want to Ack it,
> >I'll take it through mine since I have boards depending on this.
> 
> Thanks for the ack, I would like to push it via my dma-mapping tree
> together with the patches which add init_dma_coherent_pool_size()
> function. I've already added it to fixes-for-3.6 branch.

Great!

thx,

Jason.