Messages in this thread Patch in this message | | | From | Nicolin Chen <> | Subject | [RFC/RFT PATCH 1/2] dma-contiguous: Simplify dma_*_from_contiguous() function calls | Date | Mon, 29 Apr 2019 18:55:20 -0700 |
| |
Both dma_alloc_from_contiguous() and dma_release_from_contiguous() are very simply implemented, but requiring callers to pass certain parameters like count and align, and taking a boolean parameter to check __GFP_NOWARN in the allocation flags. So every function call duplicates similar work: /* A piece of example */ unsigned long order = get_order(size); size_t count = size >> PAGE_SHIFT; page = dma_alloc_from_contiguous(dev, count, order, gfp & __GFP_NOWARN); [...] dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT);
Additionally, as CMA can be used only in the context which permits sleeping, most of callers do a gfpflags_allow_blocking() check and a corresponding fallback allocation of normal pages upon any false result: /* A piece of example */ if (gfpflags_allow_blocking(flag)) page = dma_alloc_from_contiguous(); if (!page) page = alloc_pages(); [...] if (!dma_release_from_contiguous(dev, page, count)) __free_pages(page, get_order(size));
So this patch simplifies those function calls by abstracting these operations into the two functions, and drops "_from" from function names to make them shorter.
Also, for dma_release_contiguous() function specifically: a) Change it to a void type as there's no longer a need of return. b) Rename to dma_free_contiguous(), because "free" is shorter and more commonly used under kernel/dma/ folder. c) Rename one parameter from "struct page *pages" to "struct page *page", as most of callers use the naming "page" while "pages" are more oftenly used for "struct page **pages".
It then accordingly updates/cleans up all the function callers.
Note: 1) Prior to this change, amd_iommu.c has a reversed routine with alloc_pages() first and then falling back to CMA allocation. So this change may impact platforms that use amd_iommu driver. 2) This change adds an internal gfpflags_allow_blocking() check. So callers, that previously did not have this check but would potentially fail the check, might end up in the fallback path.
Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- arch/arm/mm/dma-mapping.c | 14 ++++----- arch/arm64/mm/dma-mapping.c | 11 +++---- arch/xtensa/kernel/pci-dma.c | 19 +++--------- drivers/iommu/amd_iommu.c | 20 ++++--------- drivers/iommu/intel-iommu.c | 20 ++----------- include/linux/dma-contiguous.h | 15 +++------- kernel/dma/contiguous.c | 55 ++++++++++++++++++++-------------- kernel/dma/direct.c | 24 +++------------ kernel/dma/remap.c | 11 ++----- 9 files changed, 64 insertions(+), 125 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 12e0812e988c..795e99ad36ed 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -583,12 +583,10 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size, const void *caller, bool want_vaddr, int coherent_flag, gfp_t gfp) { - unsigned long order = get_order(size); - size_t count = size >> PAGE_SHIFT; struct page *page; void *ptr = NULL; - page = dma_alloc_from_contiguous(dev, count, order, gfp & __GFP_NOWARN); + page = dma_alloc_contiguous(dev, size, gfp); if (!page) return NULL; @@ -600,7 +598,7 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size, if (PageHighMem(page)) { ptr = __dma_alloc_remap(page, size, GFP_KERNEL, prot, caller); if (!ptr) { - dma_release_from_contiguous(dev, page, count); + dma_free_contiguous(dev, page, size); return NULL; } } else { @@ -622,7 +620,7 @@ static void __free_from_contiguous(struct device *dev, struct page *page, else __dma_remap(page, size, PAGE_KERNEL); } - dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT); + dma_free_contiguous(dev, page, size); } static inline pgprot_t __get_dma_pgprot(unsigned long attrs, pgprot_t prot) @@ -1290,11 +1288,9 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) { - unsigned long order = get_order(size); struct page *page; - page = dma_alloc_from_contiguous(dev, count, order, - gfp & __GFP_NOWARN); + page = dma_alloc_contiguous(dev, size, gfp); if (!page) goto error; @@ -1369,7 +1365,7 @@ static int __iommu_free_buffer(struct device *dev, struct page **pages, int i; if (attrs & DMA_ATTR_FORCE_CONTIGUOUS) { - dma_release_from_contiguous(dev, pages[0], count); + dma_free_contiguous(dev, pages[0], size); } else { for (i = 0; i < count; i++) if (pages[i]) diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 78c0a72f822c..3f07f26d6bcd 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -158,15 +158,13 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size, pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs); struct page *page; - page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT, - get_order(size), gfp & __GFP_NOWARN); + page = dma_alloc_contiguous(dev, size, gfp); if (!page) return NULL; *handle = iommu_dma_map_page(dev, page, 0, iosize, ioprot); if (*handle == DMA_MAPPING_ERROR) { - dma_release_from_contiguous(dev, page, - size >> PAGE_SHIFT); + dma_free_contiguous(dev, page, size); return NULL; } addr = dma_common_contiguous_remap(page, size, VM_USERMAP, @@ -178,8 +176,7 @@ static void *__iommu_alloc_attrs(struct device *dev, size_t size, memset(addr, 0, size); } else { iommu_dma_unmap_page(dev, *handle, iosize, 0, attrs); - dma_release_from_contiguous(dev, page, - size >> PAGE_SHIFT); + dma_free_contiguous(dev, page, size); } } else { pgprot_t prot = arch_dma_mmap_pgprot(dev, PAGE_KERNEL, attrs); @@ -222,7 +219,7 @@ static void __iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, struct page *page = vmalloc_to_page(cpu_addr); iommu_dma_unmap_page(dev, handle, iosize, 0, attrs); - dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT); + dma_free_contiguous(dev, page, size); dma_common_free_remap(cpu_addr, size, VM_USERMAP); } else if (is_vmalloc_addr(cpu_addr)){ struct vm_struct *area = find_vm_area(cpu_addr); diff --git a/arch/xtensa/kernel/pci-dma.c b/arch/xtensa/kernel/pci-dma.c index 9171bff76fc4..b3f5acfee604 100644 --- a/arch/xtensa/kernel/pci-dma.c +++ b/arch/xtensa/kernel/pci-dma.c @@ -145,7 +145,6 @@ void __attribute__((weak)) *platform_vaddr_to_cached(void *p) void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, gfp_t flag, unsigned long attrs) { - unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; struct page *page = NULL; /* ignore region speicifiers */ @@ -155,13 +154,7 @@ void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, if (dev == NULL || (dev->coherent_dma_mask < 0xffffffff)) flag |= GFP_DMA; - if (gfpflags_allow_blocking(flag)) - page = dma_alloc_from_contiguous(dev, count, get_order(size), - flag & __GFP_NOWARN); - - if (!page) - page = alloc_pages(flag | __GFP_ZERO, get_order(size)); - + page = dma_alloc_contiguous(dev, size, flag | __GFP_ZERO); if (!page) return NULL; @@ -178,10 +171,8 @@ void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, p = dma_common_contiguous_remap(page, size, VM_MAP, pgprot_noncached(PAGE_KERNEL), __builtin_return_address(0)); - if (!p) { - if (!dma_release_from_contiguous(dev, page, count)) - __free_pages(page, get_order(size)); - } + if (!p) + dma_free_contiguous(dev, page, size); return p; } #endif @@ -193,7 +184,6 @@ void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, void arch_dma_free(struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle, unsigned long attrs) { - unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT; struct page *page; if (attrs & DMA_ATTR_NO_KERNEL_MAPPING) { @@ -207,6 +197,5 @@ void arch_dma_free(struct device *dev, size_t size, void *vaddr, page = pfn_to_page(PHYS_PFN(dma_to_phys(dev, dma_handle))); } - if (!dma_release_from_contiguous(dev, page, count)) - __free_pages(page, get_order(size)); + dma_free_contiguous(dev, page, size); } diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index f467cc4b498e..23f78e29a2d0 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -2640,16 +2640,9 @@ static void *alloc_coherent(struct device *dev, size_t size, flag &= ~(__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32); flag |= __GFP_ZERO; - page = alloc_pages(flag | __GFP_NOWARN, get_order(size)); - if (!page) { - if (!gfpflags_allow_blocking(flag)) - return NULL; - - page = dma_alloc_from_contiguous(dev, size >> PAGE_SHIFT, - get_order(size), flag & __GFP_NOWARN); - if (!page) - return NULL; - } + page = dma_alloc_contiguous(dev, size, flag | __GFP_NOWARN); + if (!page) + return NULL; if (!dma_mask) dma_mask = *dev->dma_mask; @@ -2663,9 +2656,7 @@ static void *alloc_coherent(struct device *dev, size_t size, return page_address(page); out_free: - - if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT)) - __free_pages(page, get_order(size)); + dma_free_contiguous(dev, page, size); return NULL; } @@ -2684,8 +2675,7 @@ static void free_coherent(struct device *dev, size_t size, size = PAGE_ALIGN(size); __unmap_single(dma_dom, dma_addr, size, DMA_BIDIRECTIONAL); - if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT)) - __free_pages(page, get_order(size)); + dma_free_contiguous(dev, page, size); } /* diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index d93c4bd7de75..6f94b916c386 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3787,23 +3787,13 @@ static void *intel_alloc_coherent(struct device *dev, size_t size, unsigned long attrs) { struct page *page = NULL; - int order; if (!iommu_need_mapping(dev)) return dma_direct_alloc(dev, size, dma_handle, flags, attrs); size = PAGE_ALIGN(size); - order = get_order(size); - if (gfpflags_allow_blocking(flags)) { - unsigned int count = size >> PAGE_SHIFT; - - page = dma_alloc_from_contiguous(dev, count, order, - flags & __GFP_NOWARN); - } - - if (!page) - page = alloc_pages(flags, order); + page = dma_alloc_contiguous(dev, size, flags); if (!page) return NULL; memset(page_address(page), 0, size); @@ -3813,8 +3803,7 @@ static void *intel_alloc_coherent(struct device *dev, size_t size, dev->coherent_dma_mask); if (*dma_handle != DMA_MAPPING_ERROR) return page_address(page); - if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT)) - __free_pages(page, order); + dma_free_contiguous(dev, page, size); return NULL; } @@ -3822,18 +3811,15 @@ static void *intel_alloc_coherent(struct device *dev, size_t size, static void intel_free_coherent(struct device *dev, size_t size, void *vaddr, dma_addr_t dma_handle, unsigned long attrs) { - int order; struct page *page = virt_to_page(vaddr); if (!iommu_need_mapping(dev)) return dma_direct_free(dev, size, vaddr, dma_handle, attrs); size = PAGE_ALIGN(size); - order = get_order(size); intel_unmap(dev, dma_handle, size); - if (!dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT)) - __free_pages(page, order); + dma_free_contiguous(dev, page, size); } static void intel_unmap_sg(struct device *dev, struct scatterlist *sglist, diff --git a/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h index f247e8aa5e3d..c8ae5ab968c8 100644 --- a/include/linux/dma-contiguous.h +++ b/include/linux/dma-contiguous.h @@ -111,10 +111,8 @@ static inline int dma_declare_contiguous(struct device *dev, phys_addr_t size, return ret; } -struct page *dma_alloc_from_contiguous(struct device *dev, size_t count, - unsigned int order, bool no_warn); -bool dma_release_from_contiguous(struct device *dev, struct page *pages, - int count); +struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp); +void dma_free_contiguous(struct device *dev, struct page *page, size_t size); #else @@ -144,18 +142,13 @@ int dma_declare_contiguous(struct device *dev, phys_addr_t size, } static inline -struct page *dma_alloc_from_contiguous(struct device *dev, size_t count, - unsigned int order, bool no_warn) +struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) { return NULL; } static inline -bool dma_release_from_contiguous(struct device *dev, struct page *pages, - int count) -{ - return false; -} +void dma_free_contiguous(struct device *dev, struct page *page, size_t size) { } #endif diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c index b2a87905846d..afe5a673668e 100644 --- a/kernel/dma/contiguous.c +++ b/kernel/dma/contiguous.c @@ -178,40 +178,51 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base, } /** - * dma_alloc_from_contiguous() - allocate pages from contiguous area + * dma_alloc_contiguous() - allocate contiguous pages * @dev: Pointer to device for which the allocation is performed. - * @count: Requested number of pages. - * @align: Requested alignment of pages (in PAGE_SIZE order). - * @no_warn: Avoid printing message about failed allocation. + * @size: Requested allocation size. + * @gfp: Allocation flags. * - * 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 dev_get_cma_area() helper - * function. + * This function allocates contiguous memory buffer for specified device. It + * first tries to use device specific contiguous memory area if available or + * the default global one, then tries a fallback allocation of normal pages. */ -struct page *dma_alloc_from_contiguous(struct device *dev, size_t count, - unsigned int align, bool no_warn) +struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) { - if (align > CONFIG_CMA_ALIGNMENT) - align = CONFIG_CMA_ALIGNMENT; + int node = dev ? dev_to_node(dev) : NUMA_NO_NODE; + size_t count = PAGE_ALIGN(size) >> PAGE_SHIFT; + size_t align = get_order(PAGE_ALIGN(size)); + struct cma *cma = dev_get_cma_area(dev); + struct page *page = NULL; + + /* CMA can be used only in the context which permits sleeping */ + if (cma && gfpflags_allow_blocking(gfp)) { + align = min_t(size_t, align, CONFIG_CMA_ALIGNMENT); + page = cma_alloc(cma, count, align, gfp & __GFP_NOWARN); + } + + /* Fallback allocation of normal pages */ + if (!page) + page = alloc_pages_node(node, gfp, align); - return cma_alloc(dev_get_cma_area(dev), count, align, no_warn); + return page; } /** - * dma_release_from_contiguous() - release allocated pages + * dma_free_contiguous() - release allocated pages * @dev: Pointer to device for which the pages were allocated. - * @pages: Allocated pages. - * @count: Number of allocated pages. + * @page: Pointer to the allocated pages. + * @size: Size of allocated pages. * - * This function releases memory allocated by dma_alloc_from_contiguous(). - * It returns false when provided pages do not belong to contiguous area and - * true otherwise. + * This function releases memory allocated by dma_alloc_contiguous(). As the + * cma_release returns false when provided pages do not belong to contiguous + * area and true otherwise, this function then does a fallback __free_pages() + * upon a false-return. */ -bool dma_release_from_contiguous(struct device *dev, struct page *pages, - int count) +void dma_free_contiguous(struct device *dev, struct page *page, size_t size) { - return cma_release(dev_get_cma_area(dev), pages, count); + if (!cma_release(dev_get_cma_area(dev), page, size >> PAGE_SHIFT)) + __free_pages(page, get_order(size)); } /* diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index 2c2772e9702a..0816c1e8b05a 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -96,8 +96,6 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size) struct page *__dma_direct_alloc_pages(struct device *dev, size_t size, dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs) { - unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT; - int page_order = get_order(size); struct page *page = NULL; u64 phys_mask; @@ -109,20 +107,9 @@ struct page *__dma_direct_alloc_pages(struct device *dev, size_t size, gfp |= __dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask, &phys_mask); again: - /* CMA can be used only in the context which permits sleeping */ - if (gfpflags_allow_blocking(gfp)) { - page = dma_alloc_from_contiguous(dev, count, page_order, - gfp & __GFP_NOWARN); - if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) { - dma_release_from_contiguous(dev, page, count); - page = NULL; - } - } - if (!page) - page = alloc_pages_node(dev_to_node(dev), gfp, page_order); - + page = dma_alloc_contiguous(dev, size, gfp); if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) { - __free_pages(page, page_order); + dma_free_contiguous(dev, page, size); page = NULL; if (IS_ENABLED(CONFIG_ZONE_DMA32) && @@ -154,7 +141,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size, if (PageHighMem(page)) { /* * Depending on the cma= arguments and per-arch setup - * dma_alloc_from_contiguous could return highmem pages. + * dma_alloc_contiguous could return highmem pages. * Without remapping there is no way to return them here, * so log an error and fail. */ @@ -176,10 +163,7 @@ void *dma_direct_alloc_pages(struct device *dev, size_t size, void __dma_direct_free_pages(struct device *dev, size_t size, struct page *page) { - unsigned int count = PAGE_ALIGN(size) >> PAGE_SHIFT; - - if (!dma_release_from_contiguous(dev, page, count)) - __free_pages(page, get_order(size)); + dma_free_contiguous(dev, page, size); } void dma_direct_free_pages(struct device *dev, size_t size, void *cpu_addr, diff --git a/kernel/dma/remap.c b/kernel/dma/remap.c index 2b750f13bc8f..9512684a18b8 100644 --- a/kernel/dma/remap.c +++ b/kernel/dma/remap.c @@ -107,17 +107,11 @@ early_param("coherent_pool", early_coherent_pool); int __init dma_atomic_pool_init(gfp_t gfp, pgprot_t prot) { - unsigned int pool_size_order = get_order(atomic_pool_size); - unsigned long nr_pages = atomic_pool_size >> PAGE_SHIFT; struct page *page; void *addr; int ret; - if (dev_get_cma_area(NULL)) - page = dma_alloc_from_contiguous(NULL, nr_pages, - pool_size_order, false); - else - page = alloc_pages(gfp, pool_size_order); + page = dma_alloc_contiguous(NULL, atomic_pool_size, gfp); if (!page) goto out; @@ -148,8 +142,7 @@ int __init dma_atomic_pool_init(gfp_t gfp, pgprot_t prot) gen_pool_destroy(atomic_pool); atomic_pool = NULL; free_page: - if (!dma_release_from_contiguous(NULL, page, nr_pages)) - __free_pages(page, pool_size_order); + dma_free_contiguous(NULL, page, atomic_pool_size); out: pr_err("DMA: failed to allocate %zu KiB pool for atomic coherent allocation\n", atomic_pool_size / 1024); -- 2.17.1
| |