Re: + mm-introduce-reported-pages.patch added to -mm tree

[Dave Hansen <dave.hansen@intel.com>]

On 11/6/19 4:52 PM, David Hildenbrand wrote:
>> It seems to me like Alex's approach tries to minimize new metadata, but
>> imposes some rules on the internals of the allocator.  Nitesh's is more
>> disconnected from the allocator, but has the increased complexity of
>> managing some new disjoint metadata.
>>
>> Right?
> Very right AFAIKS. It’s a matter of where to store new metadata and
> how to stop the buddy from reusing pages that are currently getting
> reported.  The latter does not spill virt specific optimization stuff
> into the core buddy, which is why that information has to be tracked
> differently.

I went back and looked at how much is getting spilled into the core
buddy.  The refactoring (patches 1-2) looks pretty neutral in terms of
complexity.  I think it actually makes things look a wee bit nicer.

Patch 3 does, indeed, poke into the core of the allocator.  There are
really six new sites that need to be poked:

	2 sites in for compaction.c
	1 site in memory_hotplug.c
	3 sites in page_alloc.c

Plus an extra function call argument and the new free_reported_page() in
page_alloc.c.  That's not _great_, of course, but it's not a deal
breaker for me, but maybe I've seen too many buddy atrocities over the
years and I'm numb to the pain. :)

Nitesh's patches seem to do this with a single poke at the allocator.
The downside is that since there are new data structures to manage, it
takes more code to create and use them since the buddy structures don't
get reused.

Both approaches seem totally reasonable to me.  I don't like Alex's
pokes into the core of the allocator, but I also don't like Nitesh's
extra data structures and the (yet unimplemented) code that it will take
to make them handle all the things they need to like hotplug.

There's also a common kernel/hypervisor ABI that's *SHARED* between the
two approaches.  That's really the only thing that would marry us to one
approach versus the other forever.

Am I missing something?  What's the harm in merging the implementation
that's ready today?  If a better one comes along, we'll rip out the ~15
lines of code in page_alloc.c and merge the better one.