Re: [PATCH 1/2] mm: cma: split out in_cma check to separate function
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2016-02-19 21:23:43
Also in:
linux-arm-kernel, lkml
On Fri, 19 Feb 2016 09:12:03 +0100 Rabin Vincent [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Split out the logic in cma_release() which checks if the page is in the contiguous area to a new function which can be called separately. ARM will use this. ...--- a/include/linux/cma.h +++ b/include/linux/cma.h@@ -27,5 +27,17 @@ extern int cma_init_reserved_mem(phys_addr_t base, phys_addr_t size, unsigned int order_per_bit, struct cma **res_cma); extern struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align); + extern bool cma_release(struct cma *cma, const struct page *pages, unsigned int count); +#ifdef CONFIG_CMA +extern bool in_cma(struct cma *cma, const struct page *pages, + unsigned int count); +#else +static inline bool in_cma(struct cma *cma, const struct page *pages, + unsigned int count) +{ + return false; +} +#endif
Calling it "pages" is weird. I immediately read it as a `struct page **'. Drop the 's' please. Or call it `start_page' if you wish to retain the "we're dealing with more than one page here" info. And `nr_pages' is a better name than `count'. And `in_cma' seems rather ... brief. And it breaks the convention that interface identifiers start with the name of the subsystem. Look at the rest of cma.h: cma_get_base(), cma_get_size() cma_declare_contiguous(), etc - let's not break that.
quoted hunk ↗ jump to hunk
#endifdiff --git a/mm/cma.c b/mm/cma.c index ea506eb..55cda16 100644 --- a/mm/cma.c +++ b/mm/cma.c@@ -426,6 +426,23 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align) return page; } +bool in_cma(struct cma *cma, const struct page *pages, unsigned int count)
A bit of documentation would be nice.
+{
+ unsigned long pfn;
+
+ if (!cma || !pages)
+ return false;Is this actually needed? If there's no good reason for the test, let's leave it out because it will just be hiding bugs in the caller.
+ pfn = page_to_pfn(pages); + + if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count) + return false; + + VM_BUG_ON(pfn + count > cma->base_pfn + cma->count); + + return true; +} + /** * cma_release() - release allocated pages * @cma: Contiguous memory region for which the allocation is performed.
Apart from those cosmeticish things, no objections from me. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>