Re: [PATCH] mm: change size_t to unsigned int for cma_alloc
From: Minchan Kim <minchan@kernel.org>
Date: 2021-03-30 15:01:03
Also in:
lkml
On Tue, Mar 30, 2021 at 09:58:37AM +0200, David Hildenbrand wrote:
On 29.03.21 22:12, Minchan Kim wrote:quoted
On Mon, Mar 29, 2021 at 07:44:31PM +0100, Matthew Wilcox wrote:quoted
On Mon, Mar 29, 2021 at 11:25:53AM -0700, Minchan Kim wrote:quoted
size_t in cma_alloc is confusing since it makes people think it's byte count, not pages. Fix it.i think it has to be unsigned long. 67a2e213e7e937c41c52ab5bc46bf3f4de469f6eRight. Fortunately, we don't have such large allocations yet via CMA/alloc_contig_rangequoted
Thanks for the pinter. I wanted to have the smallest change. The commit leads me to change cma_release, trace_cma_alloc, cma_clear_bitmap and front_contig_range as well.(Not sure we have more. Will check). Ccing david@redhat.com for upcoming changing free_contig_range.While at it, we might want to convert free_contig_range() to eat "unsigned long start, unsigned long end" like alloc_contig_range(), instead of "unsigned long pfn, unsigned int nr_pages" like alloc_contig_pages() ...
Well, I personllay tempted to change alloc_contig_range, not free_contig_range because base_pfn with nr_pages was more straightforward than base_pfn and end_pfn in that we don't need to tell whether end_pfn is inclusive or exclusive. When I look at callers of [alloc|free]_contig_range, many of them already have used nr_pages based approach rather than start_pfn, end_pfn. If your suggestion come from that "it's *range* API", I'd like to rename it with "alloc_contig_pages|free_contig_pages". Since it's beyond the goal of this patch and might be controversial, I will not deal with it in this patch.