Re: [PATCH RFC v1 01/12] mm/memory_hotplug: Don't allow to online/offline memory blocks with holes

[David Hildenbrand <david@redhat.com>]

On 24.10.19 05:53, Anshuman Khandual wrote:
> 
> On 10/22/2019 10:42 PM, David Hildenbrand wrote:
>> Our onlining/offlining code is unnecessarily complicated. Only memory
>> blocks added during boot can have holes. Hotplugged memory never has
>> holes. That memory is already online.
> 
> Why hot plugged memory at runtime cannot have holes (e.g a semi bad DIMM).

Important: HWPoison != memory hole

A memory hole is memory that is not "IORESOURCE_SYSRAM". These pages are 
currently marked PG_reserved. Such holes are sometimes used for mapping 
something into kernel space. Some archs use the PG_reserved to detect 
the memory hole ("not ram") and ignore the memmap.

Poisoned pages are marked PG_hwpoison.

> Currently, do we just abort adding that memory block if there are holes ?

There is no interface to do that.

E.g., have a look at add_memory() add_memory_resource(). You can only 
pass one memory resource (that is all IORESOURCE_SYSRAM | IORESOURCE_BUSY)

Hotplugging memory with holes is not supported (nor can I imagine a use 
case for that).

>>
>> When we stop allowing to offline memory blocks with holes, we implicitly
>> stop to online memory blocks with holes.
> 
> Reducing hotplug support for memory blocks with holes just to simplify
> the code. Is it worth ?

Me and Michal are not aware of a users, not even aware of a use case. 
Keeping code around that nobody really needs that limits cleanups, no 
thanks. Similar to us not supporting to offline memory blocks that span 
multiple nodes/zones.

E.g., have a look at the isolation code. It is full of code that jumps 
over memory holes (start_isolate_page_range() -> __first_valid_page()). 
That made sense for our complicated memory offlining code, but it is 
actually harmful when dealing with alloc_contig_range(). Allocation 
never wants to jump over memory holes. After this patch, we can just 
fail hard on any memory hole we detect, instead of ignoring it (or 
special-casing it).

> 
>>
>> This allows to simplify the code. For example, we no longer have to
>> worry about marking pages that fall into memory holes PG_reserved when
>> onlining memory. We can stop setting pages PG_reserved.
> 
> Could not there be any other way of tracking these holes if not the page
> reserved bit. In the memory section itself and corresponding struct pages
> just remained poisoned ? Just wondering, might be all wrong here.

Of course there could be ways (e.g., using PG_offline eventually), but 
it boils down to us having to deal with it in onlining/offlining code. 
And that is some handling nobody really seems to need.

> 
>>
>> Offlining memory blocks added during boot is usually not guranteed to work
>> either way. So stopping to do that (if anybody really used and tested
> 
> That guarantee does not exist right now because how boot memory could have
> been used after boot not from a limitation of the memory hot remove itself.

Yep. However, Michal and I are not even aware of a setup that would made 
this work and guarantee that the existing code actually still is able to 
deal with holes. Are you?

> 
>> this over the years) should not really hurt. For the use case of
>> offlining memory to unplug DIMMs, we should see no change. (holes on
>> DIMMs would be weird)
> 
> Holes on DIMM could be due to HW errors affecting only parts of it. By not

Again, HW errors != holes. We have PG_hwpoison for that.

> allowing such DIMM's hot add and remove, we are definitely reducing the
> scope of overall hotplug functionality. Is code simplification in itself
> is worth this reduction in functionality ?

What you describe is not affected.

Thanks!

-- 

Thanks,

David / dhildenb