Messages in this thread | | | Subject | Re: [PATCH V2] mm/page_alloc: Add alloc_contig_pages() | From | David Hildenbrand <> | Date | Thu, 17 Oct 2019 09:38:48 +0200 |
| |
On 17.10.19 09:34, Michal Hocko wrote: > On Thu 17-10-19 09:21:24, David Hildenbrand wrote: >> On 17.10.19 09:11, Michal Hocko wrote: >>> On Thu 17-10-19 10:44:41, Anshuman Khandual wrote: >>> [...] >>>> Does this add-on documentation look okay ? Should we also mention about the >>>> possible reduction in chances of success during pfn block search for the >>>> non-power-of-two cases as the implicit alignment will probably turn out to >>>> be bigger than nr_pages itself ? >>>> >>>> * Requested nr_pages may or may not be power of two. The search for suitable >>>> * memory range in a zone happens in nr_pages aligned pfn blocks. But in case >>>> * when nr_pages is not power of two, an implicitly aligned pfn block search >>>> * will happen which in turn will impact allocated memory block's alignment. >>>> * In these cases, the size (i.e nr_pages) and the alignment of the allocated >>>> * memory will be different. This problem does not exist when nr_pages is power >>>> * of two where the size and the alignment of the allocated memory will always >>>> * be nr_pages. >>> >>> I dunno, it sounds more complicated than really necessary IMHO. Callers >>> shouldn't really be bothered by memory blocks and other really deep >>> implementation details.. Wouldn't be the below sufficient? >>> >>> The allocated memory is always aligned to a page boundary. If nr_pages >>> is a power of two then the alignement is guaranteed to be to the given >> >> s/alignement/alignment/ >> >> and "the PFN is guaranteed to be aligned to nr_pages" (the address is >> aligned to nr_pages*PAGE_SIZE) > > thx for the correction. > >>> nr_pages (e.g. 1GB request would be aligned to 1GB). >>> >> >> I'd probably add "This function will miss allocation opportunities if >> nr_pages is not a power of two (and the implicit alignment is bogus)." > > This is again an implementation detail and quite a confusing one to > whoever not familiar with the MM internals. And to be fair even a proper > alignment doesn't give you any stronger guarantee as long as the > allocation operates on non movable zones anyway. >
To be honest, I'd not suggest to anyone to use this function with nr_pages not being a power of two, and I already explained why. I prefer to spill that out than having people complain afterwards. Yes it's an implementation detail users should be aware of until reworked.
But I think we talked about this here for way too long, so I am fine with either.
--
Thanks,
David / dhildenb
| |