Hi,
The commit e9da6e9 "ARM: dma-mapping: remove custom consistent dma region" breaks the compatibility with existing drivers. This causes the following kernel oops(*1). That driver has called dma_pool_alloc() to allocate memory from the interrupt context, and it hits BUG_ON(in_interrpt()) in "get_vm_area_caller()". This patch seris fixes this problem with making use of the pre-allocate atomic memory pool which DMA is using in the same way as DMA does now.
Any comment would be really appreciated.
v4: Fix plain memory allocation. (Konrad,Marek) Print nicer error message at __in_atomic_pool() (Konrad) v3: Provide a different path for IOMMU for more clean code. (Marek) atomic_pool is backed with struct page *pages[]. http://lists.linaro.org/pipermail/linaro-mm-sig/2012-August/002446.html v2: Don't modify attrs(DMA_ATTR_NO_KERNEL_MAPPING) for atomic allocation. (Marek) Modify vzalloc (KyongHo,Minchan) http://lists.linaro.org/pipermail/linaro-mm-sig/2012-August/002430.html v1: http://lists.linaro.org/pipermail/linaro-mm-sig/2012-August/002398.html
*1: [ 8.321343] ------------[ cut here ]------------ [ 8.325971] kernel BUG at kernel/mm/vmalloc.c:1322! [ 8.333615] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM [ 8.339436] Modules linked in: [ 8.342496] CPU: 0 Tainted: G W (3.4.6-00067-g5d485f7 #67) [ 8.349192] PC is at __get_vm_area_node.isra.29+0x164/0x16c [ 8.354758] LR is at get_vm_area_caller+0x4c/0x54 [ 8.359454] pc : [<c011297c>] lr : [<c011318c>] psr: 20000193 [ 8.359458] sp : c09edca0 ip : c09ec000 fp : ae278000 [ 8.370922] r10: f0000000 r9 : c011aa54 r8 : c0a26cb8 [ 8.376136] r7 : 00000001 r6 : 000000d0 r5 : 20000008 r4 : c09edca0 [ 8.382651] r3 : 00010000 r2 : 20000008 r1 : 00000001 r0 : 00001000 [ 8.389166] Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [ 8.396549] Control: 10c5387d Table: ad98c04a DAC: 00000015 .... [ 9.169162] dfa0: 412fc099 c09ec000 00000000 c000fdd8 c06df1e4 c0a1b080 00000000 00000000 [ 9.177329] dfc0: c0a235cc 8000406a 00000000 c0986818 ffffffff ffffffff c0986404 00000000 [ 9.185497] dfe0: 00000000 c09bb070 10c5387d c0a19c58 c09bb064 80008044 00000000 00000000 [ 9.193673] [<c011297c>] (__get_vm_area_node.isra.29+0x164/0x16c) from [<c011318c>] (get_vm_area_caller+0x4c/0x54) [ 9.204022] [<c011318c>] (get_vm_area_caller+0x4c/0x54) from [<c001aed8>] (__iommu_alloc_remap.isra.14+0x2c/0xfc) [ 9.214276] [<c001aed8>] (__iommu_alloc_remap.isra.14+0x2c/0xfc) from [<c001b06c>] (arm_iommu_alloc_attrs+0xc4/0xf8) [ 9.224795] [<c001b06c>] (arm_iommu_alloc_attrs+0xc4/0xf8) from [<c011aa54>] (pool_alloc_page.constprop.5+0x6c/0xf8) [ 9.235309] [<c011aa54>] (pool_alloc_page.constprop.5+0x6c/0xf8) from [<c011ab60>] (dma_pool_alloc+0x80/0x170) [ 9.245304] [<c011ab60>] (dma_pool_alloc+0x80/0x170) from [<c03cbbcc>] (tegra_build_dtd+0x48/0x14c) [ 9.254344] [<c03cbbcc>] (tegra_build_dtd+0x48/0x14c) from [<c03cbd4c>] (tegra_req_to_dtd+0x7c/0xa8) [ 9.263467] [<c03cbd4c>] (tegra_req_to_dtd+0x7c/0xa8) from [<c03cc140>] (tegra_ep_queue+0x154/0x33c) [ 9.272592] [<c03cc140>] (tegra_ep_queue+0x154/0x33c) from [<c03dd5b4>] (composite_setup+0x364/0x6d4) [ 9.281804] [<c03dd5b4>] (composite_setup+0x364/0x6d4) from [<c03dd9dc>] (android_setup+0xb8/0x14c) [ 9.290843] [<c03dd9dc>] (android_setup+0xb8/0x14c) from [<c03cd144>] (setup_received_irq+0xbc/0x270) [ 9.300053] [<c03cd144>] (setup_received_irq+0xbc/0x270) from [<c03cda64>] (tegra_udc_irq+0x2ac/0x2c4) [ 9.309353] [<c03cda64>] (tegra_udc_irq+0x2ac/0x2c4) from [<c00b5708>] (handle_irq_event_percpu+0x78/0x2e0) [ 9.319087] [<c00b5708>] (handle_irq_event_percpu+0x78/0x2e0) from [<c00b59b4>] (handle_irq_event+0x44/0x64) [ 9.328907] [<c00b59b4>] (handle_irq_event+0x44/0x64) from [<c00b8688>] (handle_fasteoi_irq+0xc4/0x16c) [ 9.338294] [<c00b8688>] (handle_fasteoi_irq+0xc4/0x16c) from [<c00b4f14>] (generic_handle_irq+0x34/0x48) [ 9.347858] [<c00b4f14>] (generic_handle_irq+0x34/0x48) from [<c000f6f4>] (handle_IRQ+0x54/0xb4) [ 9.356637] [<c000f6f4>] (handle_IRQ+0x54/0xb4) from [<c00084b0>] (gic_handle_irq+0x2c/0x60) [ 9.365068] [<c00084b0>] (gic_handle_irq+0x2c/0x60) from [<c000e900>] (__irq_svc+0x40/0x70) [ 9.373405] Exception stack(0xc09edf10 to 0xc09edf58) [ 9.378447] df00: 00000000 000f4240 00000003 00000000 [ 9.386615] df20: 00000000 e55bbc00 ef66f3ca 00000001 00000000 412fc099 c0abb9c8 00000000 [ 9.394781] df40: 3b9ac9ff c09edf58 c027a9bc c0042880 20000113 ffffffff [ 9.401396] [<c000e900>] (__irq_svc+0x40/0x70) from [<c0042880>] (tegra_idle_enter_lp3+0x68/0x78) [ 9.410272] [<c0042880>] (tegra_idle_enter_lp3+0x68/0x78) from [<c04701d4>] (cpuidle_idle_call+0xdc/0x3a4) [ 9.419922] [<c04701d4>] (cpuidle_idle_call+0xdc/0x3a4) from [<c000fdd8>] (cpu_idle+0xd8/0x134) [ 9.428612] [<c000fdd8>] (cpu_idle+0xd8/0x134) from [<c0986818>] (start_kernel+0x27c/0x2cc) [ 9.436952] Code: e1a00004 e3a04000 eb002265 eaffffe0 (e7f001f2) [ 9.443038] ---[ end trace 1b75b31a2719ed24 ]--- [ 9.447645] Kernel panic - not syncing: Fatal exception in interrupt
Hiroshi Doyu (4): ARM: dma-mapping: atomic_pool with struct page **pages ARM: dma-mapping: Refactor out to introduce __in_atomic_pool ARM: dma-mapping: Introduce __atomic_get_pages() for __iommu_get_pages() ARM: dma-mapping: IOMMU allocates pages from atomic_pool with GFP_ATOMIC
arch/arm/mm/dma-mapping.c | 91 ++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 82 insertions(+), 9 deletions(-)
struct page **pages is necessary to align with non atomic path in __iommu_get_pages(). atomic_pool() has the intialized **pages instead of just *page.
Signed-off-by: Hiroshi Doyu hdoyu@nvidia.com --- arch/arm/mm/dma-mapping.c | 17 ++++++++++++++--- 1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 1123808..51d5e2b 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -296,7 +296,7 @@ struct dma_pool { unsigned long *bitmap; unsigned long nr_pages; void *vaddr; - struct page *page; + struct page **pages; };
static struct dma_pool atomic_pool = { @@ -335,6 +335,7 @@ static int __init atomic_pool_init(void) unsigned long nr_pages = pool->size >> PAGE_SHIFT; unsigned long *bitmap; struct page *page; + struct page **pages; void *ptr; int bitmap_size = BITS_TO_LONGS(nr_pages) * sizeof(long);
@@ -342,21 +343,31 @@ static int __init atomic_pool_init(void) if (!bitmap) goto no_bitmap;
+ pages = kzalloc(nr_pages * sizeof(struct page *), GFP_KERNEL); + if (!pages) + goto no_pages; + if (IS_ENABLED(CONFIG_CMA)) ptr = __alloc_from_contiguous(NULL, pool->size, prot, &page); else ptr = __alloc_remap_buffer(NULL, pool->size, GFP_KERNEL, prot, &page, NULL); if (ptr) { + int i; + + for (i = 0; i < nr_pages; i++) + pages[i] = page + i; + spin_lock_init(&pool->lock); pool->vaddr = ptr; - pool->page = page; + pool->pages = pages; pool->bitmap = bitmap; pool->nr_pages = nr_pages; pr_info("DMA: preallocated %u KiB pool for atomic coherent allocations\n", (unsigned)pool->size / 1024); return 0; } +no_pages: kfree(bitmap); no_bitmap: pr_err("DMA: failed to allocate %u KiB pool for atomic coherent allocation\n", @@ -481,7 +492,7 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page) if (pageno < pool->nr_pages) { bitmap_set(pool->bitmap, pageno, count); ptr = pool->vaddr + PAGE_SIZE * pageno; - *ret_page = pool->page + pageno; + *ret_page = pool->pages[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",
Check the given range("start", "size") is included in "atomic_pool" or not.
Signed-off-by: Hiroshi Doyu hdoyu@nvidia.com --- arch/arm/mm/dma-mapping.c | 26 ++++++++++++++++++++------ 1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 51d5e2b..a62f552 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -503,20 +503,34 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page) return ptr; }
+static bool __in_atomic_pool(void *start, size_t size) +{ + struct dma_pool *pool = &atomic_pool; + void *end = start + size; + void *pool_start = pool->vaddr; + void *pool_end = pool->vaddr + pool->size; + + if (start < pool_start || start > pool_end) + return false; + + if (end <= pool_end) + return true; + + WARN(1, "Wrong coherent size(%p-%p) from atomic pool(%p-%p)\n", + start, end - 1, pool_start, pool_end - 1); + + return false; +} + static int __free_from_pool(void *start, size_t size) { struct dma_pool *pool = &atomic_pool; unsigned long pageno, count; unsigned long flags;
- if (start < pool->vaddr || start > pool->vaddr + pool->size) + if (!__in_atomic_pool(start, size)) return 0;
- if (start + size > pool->vaddr + pool->size) { - WARN(1, "freeing wrong coherent size from pool\n"); - return 0; - } - pageno = (start - pool->vaddr) >> PAGE_SHIFT; count = size >> PAGE_SHIFT;
Support atomic allocation in __iommu_get_pages().
Signed-off-by: Hiroshi Doyu hdoyu@nvidia.com --- arch/arm/mm/dma-mapping.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index a62f552..4ef2d7b 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -503,6 +503,15 @@ static void *__alloc_from_pool(size_t size, struct page **ret_page) return ptr; }
+static struct page **__atomic_get_pages(void *addr) +{ + struct dma_pool *pool = &atomic_pool; + struct page **pages = pool->pages; + int offs = (addr - pool->vaddr) >> PAGE_SHIFT; + + return pages + offs; +} + static bool __in_atomic_pool(void *start, size_t size) { struct dma_pool *pool = &atomic_pool; @@ -1288,6 +1297,9 @@ static struct page **__iommu_get_pages(void *cpu_addr, struct dma_attrs *attrs) { struct vm_struct *area;
+ if (__in_atomic_pool(cpu_addr, PAGE_SIZE)) + return __atomic_get_pages(cpu_addr); + if (dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs)) return cpu_addr;
Make use of the same atomic pool as DMA does, and skip a kernel page mapping which can involve sleep'able operations at allocating a kernel page table.
Signed-off-by: Hiroshi Doyu hdoyu@nvidia.com --- arch/arm/mm/dma-mapping.c | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 4ef2d7b..f5024f9 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1309,6 +1309,34 @@ static struct page **__iommu_get_pages(void *cpu_addr, struct dma_attrs *attrs) return NULL; }
+static void *__iommu_alloc_atomic(struct device *dev, size_t size, + dma_addr_t *handle) +{ + struct page *page; + void *addr; + + addr = __alloc_from_pool(size, &page); + if (!addr) + return NULL; + + *handle = __iommu_create_mapping(dev, &page, size); + if (*handle == DMA_ERROR_CODE) + goto err_mapping; + + return addr; + +err_mapping: + __free_from_pool(addr, size); + return NULL; +} + +static void __iommu_free_atomic(struct device *dev, struct page **pages, + dma_addr_t handle, size_t size) +{ + __iommu_remove_mapping(dev, handle, size); + __free_from_pool(page_address(pages[0]), size); +} + static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) { @@ -1326,6 +1354,9 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, *handle = DMA_ERROR_CODE; size = PAGE_ALIGN(size);
+ if (gfp & GFP_ATOMIC) + return __iommu_alloc_atomic(dev, size, handle); + pages = __iommu_alloc_buffer(dev, size, gfp); if (!pages) return NULL; @@ -1392,6 +1423,11 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, return; }
+ if (__in_atomic_pool(cpu_addr, size)) { + __iommu_free_atomic(dev, pages, handle, size); + return; + } + if (!dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs)) { unmap_kernel_range((unsigned long)cpu_addr, size); vunmap(cpu_addr);
Hello,
On 8/28/2012 7:13 AM, Hiroshi Doyu wrote:
The commit e9da6e9 "ARM: dma-mapping: remove custom consistent dma region" breaks the compatibility with existing drivers. This causes the following kernel oops(*1). That driver has called dma_pool_alloc() to allocate memory from the interrupt context, and it hits BUG_ON(in_interrpt()) in "get_vm_area_caller()". This patch seris fixes this problem with making use of the pre-allocate atomic memory pool which DMA is using in the same way as DMA does now.
Any comment would be really appreciated.
Looks fine now. I will do some tests and apply them to my fixes-for-3.6 branch. Thanks for Your contribution!
Best regards
linaro-mm-sig@lists.linaro.org
-
Hiroshi Doyu -
Marek Szyprowski