On 21.10.19 16:43, Michal Hocko wrote:
On Mon 21-10-19 16:19:25, David Hildenbrand wrote:
We call __offline_isolated_pages() from __offline_pages() after all
pages were isolated and are either free (PageBuddy()) or PageHWPoison.
Nothing can stop us from offlining memory at this point.
In __offline_isolated_pages() we first set all affected memory sections
offline (offline_mem_sections(pfn, end_pfn)), to mark the memmap as
invalid (pfn_to_online_page() will no longer succeed), and then walk over
all pages to pull the free pages from the free lists (to the isolated
free lists, to be precise).
Note that re-onlining a memory block will result in the whole memmap
getting reinitialized, overwriting any old state. We already poision the
memmap when offlining is complete to find any access to
stale/uninitialized memmaps.
So, setting the pages PageReserved() is not helpful. The memap is marked
offline and all pageblocks are isolated. As soon as offline, the memmap
is stale either way.
This looks like a leftover from ancient times where we initialized the
memmap when adding memory and not when onlining it (the pages were set
PageReserved so re-onling would work as expected).
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxx>
Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: Wei Yang <richard.weiyang@xxxxxxxxx>
Cc: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
Cc: Anshuman Khandual <anshuman.khandual@xxxxxxx>
Cc: Pavel Tatashin <pavel.tatashin@xxxxxxxxxxxxx>
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
We still set PageReserved before onlining pages and that one should be
good to go as well (memmap_init_zone).
Thanks!
memmap_init_zone() is called when onlining memory. There, set all pages
to reserved right now (on context == MEMMAP_HOTPLUG). We clear
PG_reserved when onlining a page to the buddy (e.g.,
generic_online_page). If we would online a memory block with holes, we
would want to keep all such pages (!pfn_valid()) set to reserved. Also,
there might be other side effects.
So it might not be that easy to remove. A cleanup that I have on my list
is to disallow offlining memory blocks with holes. This implies that we
will never online memory blocks with holes. This allows for some
cleanups in the onlining/offlining code. For example, it would allow to
get rid of this PG_reserved initialization.
I don't think that we have to support offlining memory blocks with
holes. This can only be bootmem (never hotplugged memory), where the
chance for this to work is in my opinion already not too good.
What's your opinion on this?
There is a comment above offline_isolated_pages_cb that should be
removed as well.
Right, I'll convert that comment
"Mark all sections offline and remove all free pages from the buddy."
Thanks!
--
Thanks,
David / dhildenb
