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

[David Hildenbrand <david@xxxxxxxxxx>]

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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel