Re: [PATCH v2 8/9] mm: cma: add __cma_release()

jane.chu
2025-09-09T00:15:58Z

On 9/2/2025 5:48 AM, Kefeng Wang wrote:
> Kill cma_pages_valid() which only used in cma_release(), also
> cleanup code duplication between cma pages valid checking and
> cma memrange finding, add __cma_release() helper to prepare for
> the upcoming frozen page release.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
> include/linux/cma.h | 1 -
> mm/cma.c | 57 ++++++++++++---------------------------------
> 2 files changed, 15 insertions(+), 43 deletions(-)
>
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> index 62d9c1cf6326..e5745d2aec55 100644
> --- a/include/linux/cma.h
> +++ b/include/linux/cma.h
> @@ -49,7 +49,6 @@ extern int cma_init_reserved_mem(phys_addr_t base, phys_addr_t size,
> struct cma **res_cma);
> extern struct page *cma_alloc(struct cma *cma, unsigned long count, unsigned int align,
> bool no_warn);
> -extern bool cma_pages_valid(struct cma *cma, const struct page *pages, unsigned long count);
> extern bool cma_release(struct cma *cma, const struct page *pages, unsigned long count);
>
> extern int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data);
> diff --git a/mm/cma.c b/mm/cma.c
> index 3f3c96be67f7..b4413e382d5d 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -936,34 +936,36 @@ struct folio *cma_alloc_folio(struct cma *cma, int order, gfp_t gfp)
> return page ? page_folio(page) : NULL;
> }
>
> -bool cma_pages_valid(struct cma *cma, const struct page *pages,
> - unsigned long count)
> +static bool __cma_release(struct cma *cma, const struct page *pages,
> + unsigned long count)
> {
> unsigned long pfn, end;
> int r;
> struct cma_memrange *cmr;
> - bool ret;
> +
> + pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
>
> if (!cma || !pages || count > cma->count)
> return false;
>
> pfn = page_to_pfn(pages);
> - ret = false;
>
> for (r = 0; r < cma->nranges; r++) {
> cmr = &cma->ranges[r];
> end = cmr->base_pfn + cmr->count;
> - if (pfn >= cmr->base_pfn && pfn < end) {
> - ret = pfn + count <= end;
> + if (pfn >= cmr->base_pfn && pfn < end && pfn + count <= end)
> break;
> - }
> }

The only difference from the previous code is the now missing VM_BUG_ON
upon a given range that stretches across the CMA ranges.
The warning was introduced by
c64be2bb1c6eb drivers: add Contiguous Memory Allocator
about 15 years ago as a caution only.
I am okay to just return false in the potential error case.

>
> - if (!ret)
> - pr_debug("%s(page %p, count %lu)\n",
> - __func__, (void *)pages, count);
> + if (r == cma->nranges)
> + return false;
>
> - return ret;
> + free_contig_range(pfn, count);
> + cma_clear_bitmap(cma, cmr, pfn, count);
> + cma_sysfs_account_release_pages(cma, count);
> + trace_cma_release(cma->name, pfn, pages, count);
> +
> + return true;
> }
>
> /**
> @@ -979,36 +981,7 @@ bool cma_pages_valid(struct cma *cma, const struct page *pages,
> bool cma_release(struct cma *cma, const struct page *pages,
> unsigned long count)
> {
> - struct cma_memrange *cmr;
> - unsigned long pfn, end_pfn;
> - int r;
> -
> - pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
> -
> - if (!cma_pages_valid(cma, pages, count))
> - return false;
> -
> - pfn = page_to_pfn(pages);
> - end_pfn = pfn + count;
> -
> - for (r = 0; r < cma->nranges; r++) {
> - cmr = &cma->ranges[r];
> - if (pfn >= cmr->base_pfn &&
> - pfn < (cmr->base_pfn + cmr->count)) {
> - VM_BUG_ON(end_pfn > cmr->base_pfn + cmr->count);
> - break;
> - }
> - }
> -
> - if (r == cma->nranges)
> - return false;
> -
> - free_contig_range(pfn, count);
> - cma_clear_bitmap(cma, cmr, pfn, count);
> - cma_sysfs_account_release_pages(cma, count);
> - trace_cma_release(cma->name, pfn, pages, count);
> -
> - return true;
> + return __cma_release(cma, pages, count);
> }
>
> bool cma_free_folio(struct cma *cma, const struct folio *folio)
> @@ -1016,7 +989,7 @@ bool cma_free_folio(struct cma *cma, const struct folio *folio)
> if (WARN_ON(!folio_test_large(folio)))
> return false;
>
> - return cma_release(cma, &folio->page, folio_nr_pages(folio));
> + return __cma_release(cma, &folio->page, folio_nr_pages(folio));
> }
>
> int cma_for_each_area(int (*it)(struct cma *cma, void *data), void *data)

Nice clean up.

Reviewed-by: Jane Chu <jane.chu@oracle.com>

-jane