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(-)
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. */
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);
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.
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
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=".
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);
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;
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.
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
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@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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
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.
linaro-mm-sig@lists.linaro.org
-
Aaro Koskinen -
Hiroshi Doyu -
Jason Cooper -
Marek Szyprowski