On Thu, Mar 1, 2018 at 5:20 PM, Daniel Vacek <neelx@xxxxxxxxxx> wrote: > On Thu, Mar 1, 2018 at 4:27 PM, Michal Hocko <mhocko@xxxxxxxxxx> wrote: >> It is still not clear why not to do the alignment in >> memblock_next_valid_pfn rather than its caller. > > As it's the mem init which needs it to be aligned. Other callers may > not, possibly? > Not that there are any other callers at the moment so it really does > not matter where it is placed. The only difference would be the end of > the loop with end_pfn vs aligned end_pfn. And it looks like the pure > (unaligned) end_pfn would be preferred here. Wanna me send a v2? Thinking about it again memblock has nothing to do with pageblock. And the function name suggests one shall get a next valid pfn, not something totally unrelated to memblock. So that's what it returns. It's the mem init which needs to align this and hence mem init aligns it for it's purposes. I'd call this the correct design. To deal with the end_pfn special case I'd actually get rid of it completely and hardcode -1UL as max pfn instead (rather than 0). Caller should handle max pfn as an error or end of the loop as here in this case. I'll send a v2 with this implemented. Paul> Why is it based on memblock actually? Wouldn't a generic mem_section solution work satisfiable for you? That would be natively aligned with whole section (doing a bit more work as a result in the end) and also independent of CONFIG_HAVE_MEMBLOCK_NODE_MAP availability. >> -- >> Michal Hocko >> SUSE Labs