Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added memory range

From: David Hildenbrand <david@redhat.com>
To: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Pavel Tatashin <pasha.tatashin@soleen.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 1/5] mm,memory_hotplug: Allocate memmap from the added memory range
Date: Thu, 25 Mar 2021 19:08:05 +0100	[thread overview]
Message-ID: <5a755ff6-4085-da64-08d5-49dd232029eb@redhat.com> (raw)
In-Reply-To: <YFy8ARml4R7/snVs@dhcp22.suse.cz>

On 25.03.21 17:36, Michal Hocko wrote:
> On Thu 25-03-21 17:20:23, David Hildenbrand wrote:
>> On 25.03.21 17:07, Michal Hocko wrote:
>>> On Thu 25-03-21 16:35:58, Michal Hocko wrote:
>>> [...]
>>>> So there is indeed a difference. One way around that would be to mark
>>>> vmemmap pages (e.g. PageReserved && magic value stored somewhere in the
>>>> struct page - resembling bootmem vmemmaps) or mark section fully backing
>>>> vmemmaps as online (ugly).
>>>
>>> I am not yet ready to give up on this. Here is a quick stab at the
>>> pfn_to_online_page approach. It is not great but it is not really
>>> terrible either. I think we can do better and skip
>>
>> We both seem to have a different taste, to phrase it in a nice way :) ; but
>> well, you seem to have set your mind (just like I seem to have set mine when
>> trying to find a nice and somewhat-clean way to handle this when discussing
>> it in the past).
> 
> I definitely do not want to fight for a certain solution just for the
> sake of it. I really dislike how the lifetime of the reserved space and
> its accounting are completely detached. But hey, I do understand that
> a worse solution from the design perspective can be better due to
> practical reasons or constrains.
> 
> I haven't seen the hibernation problem before and I do recognize it is
> a nasty one. If all it takes is to make pfn_to_online_page work (and my
> previous attempt is incorrect because it should consult block rather
> than section pfn range) and there are no other downsides then I would
> still prefer to go with my proposal.  If there are still other things to
> plug then, well, practicality is going to win.
> 
> So before I give up on the "proper" design card, are there more
> subtleties to watch for? You have certainly given this much more thought
> than I have.
> 

"Just one more thing" :)

With the pfn_to_online_page() change, I think what remains is

1. The contiguous zone thingy, which we discussed is not a deal breaker, 
although sub-optimal and most probably not to be optimized in the future.

2. There corner cases issue with /dev/mem use case with offline memory 
blocks I mentioned. Existing setups (!memmap_on_memory) are not 
affected, so I guess we're fine.

3. valid_zones_show() has to be taught to only look at the !vmemmap 
part, otherwise we'll no longer indicate "movable" after onlining to the 
movable zone. Should be fairly easy.

We'll have pfn_to_online_section() succeed without SECTION_IS_ONLINE. I 
think I/we removed all such code that purely relied on that flag for 
optimizations like

if (!online_section(s))
	continue;

I can give it some more thought, it could fly. At least zone shrinking 
and hibernation should continue working as expected, which is a relief.

-- 
Thanks,

David / dhildenb