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
