From 7ee793a62fa8c544f8b844e6e87b2d8e8836b219 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Tue, 25 Feb 2014 11:01:19 -0800 Subject: cma: Remove potential deadlock situation CMA locking is currently very coarse. The cma_mutex protects both the bitmap and avoids concurrency with alloc_contig_range. There are several situations which may result in a deadlock on the CMA mutex currently, mostly involving AB/BA situations with alloc and free. Fix this issue by protecting the bitmap with a mutex per CMA region and use the existing mutex for protecting against concurrency with alloc_contig_range. Signed-off-by: Laura Abbott Signed-off-by: Marek Szyprowski diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index 165c2c2..fe72bac 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -37,6 +37,7 @@ struct cma { unsigned long base_pfn; unsigned long count; unsigned long *bitmap; + struct mutex lock; }; struct cma *dma_contiguous_default_area; @@ -161,6 +162,7 @@ static int __init cma_activate_area(struct cma *cma) init_cma_reserved_pageblock(pfn_to_page(base_pfn)); } while (--i); + mutex_init(&cma->lock); return 0; } @@ -261,6 +263,13 @@ err: return ret; } +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count) +{ + mutex_lock(&cma->lock); + bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count); + mutex_unlock(&cma->lock); +} + /** * dma_alloc_from_contiguous() - allocate pages from contiguous area * @dev: Pointer to device for which the allocation is performed. @@ -294,30 +303,41 @@ struct page *dma_alloc_from_contiguous(struct device *dev, int count, mask = (1 << align) - 1; - mutex_lock(&cma_mutex); for (;;) { + mutex_lock(&cma->lock); pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count, start, count, mask); - if (pageno >= cma->count) + if (pageno >= cma->count) { + mutex_unlock(&cma_mutex); break; + } + bitmap_set(cma->bitmap, pageno, count); + /* + * It's safe to drop the lock here. We've marked this region for + * our exclusive use. If the migration fails we will take the + * lock again and unmark it. + */ + mutex_unlock(&cma->lock); pfn = cma->base_pfn + pageno; + mutex_lock(&cma_mutex); ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA); + mutex_unlock(&cma_mutex); if (ret == 0) { - bitmap_set(cma->bitmap, pageno, count); page = pfn_to_page(pfn); break; } else if (ret != -EBUSY) { + clear_cma_bitmap(cma, pfn, count); break; } + clear_cma_bitmap(cma, pfn, count); pr_debug("%s(): memory range at %p is busy, retrying\n", __func__, pfn_to_page(pfn)); /* try again with a bit different memory target */ start = pageno + mask + 1; } - mutex_unlock(&cma_mutex); pr_debug("%s(): returned %p\n", __func__, page); return page; } @@ -350,10 +370,8 @@ bool dma_release_from_contiguous(struct device *dev, struct page *pages, VM_BUG_ON(pfn + count > cma->base_pfn + cma->count); - mutex_lock(&cma_mutex); - bitmap_clear(cma->bitmap, pfn - cma->base_pfn, count); free_contig_range(pfn, count); - mutex_unlock(&cma_mutex); + clear_cma_bitmap(cma, pfn, count); return true; } -- cgit v0.10.2 From 006f841db1e0da8801aedc6751b563b28f9a6319 Mon Sep 17 00:00:00 2001 From: Ritesh Harjani Date: Tue, 20 May 2014 10:02:59 +0530 Subject: arm: dma-iommu: Clean up redundant variable mapping->size can be derived from mapping->bits << PAGE_SHIFT which makes mapping->size as redundant. Clean this up. Signed-off-by: Ritesh Harjani Reported-by: Will Deacon Signed-off-by: Marek Szyprowski diff --git a/arch/arm/include/asm/dma-iommu.h b/arch/arm/include/asm/dma-iommu.h index eec0a12..8e3fcb9 100644 --- a/arch/arm/include/asm/dma-iommu.h +++ b/arch/arm/include/asm/dma-iommu.h @@ -18,7 +18,6 @@ struct dma_iommu_mapping { unsigned int extensions; size_t bitmap_size; /* size of a single bitmap */ size_t bits; /* per bitmap */ - unsigned int size; /* per bitmap */ dma_addr_t base; spinlock_t lock; diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 6b00be1..3d43c41 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1074,6 +1074,7 @@ static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping, unsigned int order = get_order(size); unsigned int align = 0; unsigned int count, start; + size_t mapping_size = mapping->bits << PAGE_SHIFT; unsigned long flags; dma_addr_t iova; int i; @@ -1119,7 +1120,7 @@ static inline dma_addr_t __alloc_iova(struct dma_iommu_mapping *mapping, } spin_unlock_irqrestore(&mapping->lock, flags); - iova = mapping->base + (mapping->size * i); + iova = mapping->base + (mapping_size * i); iova += start << PAGE_SHIFT; return iova; @@ -1129,6 +1130,7 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping, dma_addr_t addr, size_t size) { unsigned int start, count; + size_t mapping_size = mapping->bits << PAGE_SHIFT; unsigned long flags; dma_addr_t bitmap_base; u32 bitmap_index; @@ -1136,14 +1138,14 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping, if (!size) return; - bitmap_index = (u32) (addr - mapping->base) / (u32) mapping->size; + bitmap_index = (u32) (addr - mapping->base) / (u32) mapping_size; BUG_ON(addr < mapping->base || bitmap_index > mapping->extensions); - bitmap_base = mapping->base + mapping->size * bitmap_index; + bitmap_base = mapping->base + mapping_size * bitmap_index; start = (addr - bitmap_base) >> PAGE_SHIFT; - if (addr + size > bitmap_base + mapping->size) { + if (addr + size > bitmap_base + mapping_size) { /* * The address range to be freed reaches into the iova * range of the next bitmap. This should not happen as @@ -1964,7 +1966,6 @@ arm_iommu_create_mapping(struct bus_type *bus, dma_addr_t base, size_t size) mapping->extensions = extensions; mapping->base = base; mapping->bits = BITS_PER_BYTE * bitmap_size; - mapping->size = mapping->bits << PAGE_SHIFT; spin_lock_init(&mapping->lock); -- cgit v0.10.2 From e464ef16c4f080bb85ccf9085f92503b5b5b1e13 Mon Sep 17 00:00:00 2001 From: Gioh Kim Date: Thu, 22 May 2014 13:38:23 +0900 Subject: arm: dma-mapping: add checking cma area initialized If CMA is turned on and CMA size is set to zero, kernel should behave as if CMA was not enabled at compile time. Every dma allocation should check existence of cma area before requesting memory. Signed-off-by: Gioh Kim Signed-off-by: Joonsoo Kim Acked-by: Michal Nazarewicz [mszyprow: removed redundant empty line from the patch] Signed-off-by: diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 3d43c41..5bef8585 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -390,7 +390,7 @@ static int __init atomic_pool_init(void) if (!pages) goto no_pages; - if (IS_ENABLED(CONFIG_DMA_CMA)) + if (dev_get_cma_area(NULL)) ptr = __alloc_from_contiguous(NULL, pool->size, prot, &page, atomic_pool_init); else @@ -701,7 +701,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, addr = __alloc_simple_buffer(dev, size, gfp, &page); else if (!(gfp & __GFP_WAIT)) addr = __alloc_from_pool(size, &page); - else if (!IS_ENABLED(CONFIG_DMA_CMA)) + else if (!dev_get_cma_area(dev)) addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller); else addr = __alloc_from_contiguous(dev, size, prot, &page, caller); @@ -790,7 +790,7 @@ static void __arm_dma_free(struct device *dev, size_t size, void *cpu_addr, __dma_free_buffer(page, size); } else if (__free_from_pool(cpu_addr, size)) { return; - } else if (!IS_ENABLED(CONFIG_DMA_CMA)) { + } else if (!dev_get_cma_area(dev)) { __dma_free_remap(cpu_addr, size); __dma_free_buffer(page, size); } else { -- cgit v0.10.2 From bb56d0dc23aa3428c0b1901414bfcf698eb693fb Mon Sep 17 00:00:00 2001 From: Gioh Kim Date: Thu, 22 May 2014 13:31:37 +0900 Subject: drivers/base/dma-contiguous.c: erratum of dev_get_cma_area fix erratum get_dev_cma_area into dev_get_cma_area Signed-off-by: Gioh Kim Signed-off-by: Marek Szyprowski diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index fe72bac..7b0217c 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -278,7 +278,7 @@ static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count) * * This function allocates memory buffer for specified device. It uses * device specific contiguous memory area if available or the default - * global one. Requires architecture specific get_dev_cma_area() helper + * global one. Requires architecture specific dev_get_cma_area() helper * function. */ struct page *dma_alloc_from_contiguous(struct device *dev, int count, -- cgit v0.10.2 From f70e3c4f8b6ab61f713e040822ec51f5de498146 Mon Sep 17 00:00:00 2001 From: Joonsoo Kim Date: Thu, 29 May 2014 15:29:18 +0900 Subject: CMA: correct unlock target 'cma: Remove potential deadlock situation' introduces per cma area mutex for bitmap management. It is good, but there is one mistake. When we can't find appropriate area in bitmap, we release cma_mutex global lock rather than cma->lock and this is a bug. So fix it. Signed-off-by: Joonsoo Kim Signed-off-by: Marek Szyprowski diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c index 7b0217c..c34ec33 100644 --- a/drivers/base/dma-contiguous.c +++ b/drivers/base/dma-contiguous.c @@ -309,7 +309,7 @@ struct page *dma_alloc_from_contiguous(struct device *dev, int count, pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count, start, count, mask); if (pageno >= cma->count) { - mutex_unlock(&cma_mutex); + mutex_unlock(&cma->lock); break; } bitmap_set(cma->bitmap, pageno, count); -- cgit v0.10.2