mm: Memory offlining + page isolation cleanups
Subsystems (7)
v2 cover mm: Memory offlining + page isolation cleanups
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463
Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of PG_reserved during memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.
v1 -> v2:
- "mm/page_alloc.c: Don't set pages PageReserved() when offlining" -- Fixup one comment
- "mm/page_isolation.c: Convert SKIP_HWPOISON to MEMORY_OFFLINE" -- Use parenthesis around checks
- Added ACKs
David Hildenbrand (2):
mm/page_alloc.c: Don't set pages PageReserved() when offlining mm/page_isolation.c: Convert SKIP_HWPOISON to MEMORY_OFFLINE
include/linux/page-isolation.h | 4 ++-- mm/memory_hotplug.c | 12 ++++++------ mm/page_alloc.c | 9 +++------ mm/page_isolation.c | 12 ++++++------ 4 files changed, 17 insertions(+), 20 deletions(-)
Signature · 1 line
2.21.0
PATCH v2 1/2 mm/page_alloc.c: Don't set pages PageReserved() when offlining
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).
--- mm/memory_hotplug.c | 4 +--- mm/page_alloc.c | 5 +---- 2 files changed, 2 insertions(+), 7 deletions(-)
mm/memory_hotplug.c +1 -3
@@ -1346,9 +1346,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)return ret; } -/* -* remove from free_area[] and mark all as Reserved. -*/ +/* Mark all sections offline and remove all free pages from the buddy. */ static int offline_isolated_pages_cb (unsigned long start,unsigned long nr_pages,void *data)
mm/page_alloc.c +1 -4
@@ -8667,7 +8667,7 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn) {struct page *page;struct zone *zone; -unsigned int order, i; +unsigned int order;unsigned long pfn;unsigned long flags;unsigned long offlined_pages =0 ;
@@ -8695,7 +8695,6 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)*/ if (unlikely (!PageBuddy (page) &&PageHWPoison (page))) { pfn++; -SetPageReserved (page); offlined_pages++;continue ; }
@@ -8709,8 +8708,6 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn) pfn, 1 << order, end_pfn);#endif del_page_from_free_area (page, &zone->free_area[order]); -for (i =0 ; i < (1 << order); i++) -SetPageReserved ((page+i)); pfn += (1 << order); }spin_unlock_irqrestore (&zone->lock, flags);
Signature · 1 line
2.21.0
PATCH v2 2/2 mm/page_isolation.c: Convert SKIP_HWPOISON to MEMORY_OFFLINE
In reply to David HildenbrandWe have two types of users of page isolation:
1. Memory offlining: Offline memory so it can be unplugged. Memory won't be touched.
2. Memory allocation: Allocate memory (e.g., alloc_contig_range()) to become the owner of the memory and make use of it.
For example, in case we want to offline memory, we can ignore (skip over) PageHWPoison() pages, as the memory won't get used. We can allow to offline memory. In contrast, we don't want to allow to allocate such memory.
Let's generalize the approach so we can special case other types of pages we want to skip over in case we offline memory. While at it, also pass the same flags to test_pages_isolated().
--- include/linux/page-isolation.h | 4 ++-- mm/memory_hotplug.c | 8 +++++--- mm/page_alloc.c | 4 ++-- mm/page_isolation.c | 12 ++++++------ 4 files changed, 15 insertions(+), 13 deletions(-)
include/linux/page-isolation.h +2 -2
@@ -30,7 +30,7 @@ static inline bool is_migrate_isolate(int migratetype) }#endif -#define SKIP_HWPOISON 0x1 +#define MEMORY_OFFLINE 0x1#define REPORT_FAILURE 0x2 bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
@@ -58,7 +58,7 @@ undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,* Test all pages in [start_pfn, end_pfn) are isolated or not. */ int test_pages_isolated (unsigned long start_pfn,unsigned long end_pfn, -bool skip_hwpoisoned_pages); +int isol_flags);struct page *alloc_migrate_target (struct page *page,unsigned long private);
mm/memory_hotplug.c +5 -3
@@ -1149,7 +1149,8 @@ static bool is_pageblock_removable_nolock(unsigned long pfn)if (!zone_spans_pfn (zone, pfn))return false; -return !has_unmovable_pages (zone, page,0 ,MIGRATE_MOVABLE ,SKIP_HWPOISON ); +return !has_unmovable_pages (zone, page,0 ,MIGRATE_MOVABLE , +MEMORY_OFFLINE ); }/* Checks if this range of memory is likely to be hot-removable. */
@@ -1364,7 +1365,8 @@ static int check_pages_isolated_cb(unsigned long start_pfn, unsigned long nr_pages, void *data) { -return test_pages_isolated (start_pfn, start_pfn + nr_pages, true); +return test_pages_isolated (start_pfn, start_pfn + nr_pages, +MEMORY_OFFLINE ); }static int __init cmdline_parse_movable_node (char *p)
@@ -1475,7 +1477,7 @@ static int __ref __offline_pages(unsigned long start_pfn,/* set above range as isolated */ ret =start_isolate_page_range (start_pfn, end_pfn,MIGRATE_MOVABLE , -SKIP_HWPOISON |REPORT_FAILURE ); +MEMORY_OFFLINE |REPORT_FAILURE );if (ret <0 ) { reason ="failure to isolate range" ; goto failed_removal;
mm/page_alloc.c +2 -2
@@ -8270,7 +8270,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,* The HWPoisoned page may be not in buddy system, and * page_count() is not 0. */ -if ((flags &SKIP_HWPOISON ) &&PageHWPoison (page)) +if ((flags &MEMORY_OFFLINE ) &&PageHWPoison (page))continue ;if (__PageMovable (page))
@@ -8486,7 +8486,7 @@ int alloc_contig_range(unsigned long start, unsigned long end, }/* Make sure the range is really isolated. */ -if (test_pages_isolated (outer_start, end, false)) { +if (test_pages_isolated (outer_start, end,0 )) {pr_info_ratelimited ("%s: [%lx, %lx) PFNs busy\n" , __func__, outer_start, end); ret = -EBUSY ;
mm/page_isolation.c +6 -6
@@ -168,7 +168,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)* @migratetype: Migrate type to set in error recovery. * @flags: The following flags are allowed (they can be combined in * a bit mask) -* SKIP_HWPOISON - ignore hwpoison pages +* MEMORY_OFFLINE - isolate to offline (!allocate) memory +* e.g., skip over PageHWPoison() pages * REPORT_FAILURE - report details about the failure to * isolate the range *
@@ -257,7 +258,7 @@ void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,*/ static unsigned long __test_page_isolated_in_pageblock (unsigned long pfn,unsigned long end_pfn, -bool skip_hwpoisoned_pages) +int flags) {struct page *page;
@@ -274,7 +275,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,* simple way to verify that as VM_BUG_ON(), though. */ pfn +=1 <<page_order (page); - else if (skip_hwpoisoned_pages && PageHWPoison(page)) + else if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))/* A HWPoisoned page cannot be also PageBuddy */ pfn++; else
@@ -286,7 +287,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,/* Caller should ensure that requested range is in a single zone */ int test_pages_isolated (unsigned long start_pfn,unsigned long end_pfn, -bool skip_hwpoisoned_pages) +int isol_flags) {unsigned long pfn, flags;struct page *page;
@@ -308,8 +309,7 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,/* Check all pages are free or marked as ISOLATED */ zone =page_zone (page);spin_lock_irqsave (&zone->lock, flags); - pfn =__test_page_isolated_in_pageblock (start_pfn, end_pfn, - skip_hwpoisoned_pages); + pfn =__test_page_isolated_in_pageblock (start_pfn, end_pfn, isol_flags);spin_unlock_irqrestore (&zone->lock, flags);trace_test_pages_isolated (start_pfn, end_pfn, pfn);
Signature · 1 line
2.21.0
Quoted 9 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.
BTW, I remember that ZONE_DEVICE pages are still required to be set PG_reserved. That has to be sorted out first. I remember that somebody was working on it a while ago but didn't hear about that again. Will look into that as well - should be as easy as adding a zone check (if there isn't a pfn_to_online_page() check already). But of course, there might be special cases ....
Signature · 3 lines
Thanks, David / dhildenb
Quoted 6 lines · David Hildenbrand
Quoted 5 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
Quoted 7 lines
Quoted 4 lines
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.BTW, I remember that
ZONE_DEVICEpages are still required to be setPG_reserved. That has to be sorted out first.
Do they?
Quoted 5 lines
I remember that somebody was
working on it a while ago but didn't hear about that again. Will look into that as well - should be as easy as adding a zone check (if there isn't apfn_to_online_page()check already). But of course, there might be special cases ....
I remember Alexander didn't want to change the PageReserved handling because he was worried about unforeseeable side effects. I have a vague recollection he (or maybe Dan) has promissed some follow up clean ups which didn't seem to materialize.
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 12 lines · Michal Hocko
Quoted 11 lines · David Hildenbrand
Quoted 9 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.
Quoted 4 lines
Quoted 2 lines
BTW, I remember that
ZONE_DEVICEpages are still required to be setPG_reserved. That has to be sorted out first.Do they?
Yes, especially KVM code :/
Quoted 11 lines
Quoted 6 lines
I remember that somebody was
working on it a while ago but didn't hear about that again. Will look into that as well - should be as easy as adding a zone check (if there isn't apfn_to_online_page()check already). But of course, there might be special cases ....I remember Alexander didn't want to change the PageReserved handling because he was worried about unforeseeable side effects. I have a vague recollection he (or maybe Dan) has promissed some follow up clean ups which didn't seem to materialize.
I'm looking into it right now, especially the KVM part.
Signature · 3 lines
Thanks, David / dhildenb
Quoted 13 lines · David Hildenbrand
Quoted 12 lines · Michal Hocko
Quoted 11 lines · David Hildenbrand
Quoted 9 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
BTW, I remember that
ZONE_DEVICEpages are still required to be setPG_reserved. That has to be sorted out first.Do they?
Yes, especially KVM code :/
Details please?
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 14 lines · Michal Hocko
Quoted 13 lines · David Hildenbrand
Quoted 12 lines · Michal Hocko
Quoted 11 lines · David Hildenbrand
Quoted 9 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
BTW, I remember that
ZONE_DEVICEpages are still required to be setPG_reserved. That has to be sorted out first.Do they?
Yes, especially KVM code :/
Details please?
E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()
And I currently have
From 55606751b67989bd06d17844a6bcfbf85d44ee69 Mon Sep 17 00:00:00 2001
From: David Hildenbrand <[email protected]>
Date: Tue, 22 Oct 2019 10:08:04 +0200
Subject: [PATCH v1] KVM: x86/mmu: Prepare kvm_is_mmio_pfn() for PG_reserved
changes
Right now, ZONE_DEVICE memory is always set PG_reserved. We want to change that in the future.
KVM has this weird use case that you can map anything from /dev/mem into the guest. pfn_valid() is not a reliable check whether the memmap was initialized and can be touched. pfn_to_online_page() makes sure that we have an initialized memmap - however, there is no reliable and fast check to detect memmaps that were initialized and are ZONE_DEVICE.
Let's rewrite kvm_is_mmio_pfn() so we really only touch initialized memmaps that are guaranteed to not contain garbage. Make sure that RAM without a memmap is still not detected as MMIO and that ZONE_DEVICE that is not UC/UC-/WC is not detected as MMIO.
--- arch/x86/kvm/mmu.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-)
arch/x86/kvm/mmu.c +22 -16
@@ -2962,23 +2962,29 @@ static bool mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn,static bool kvm_is_mmio_pfn (kvm_pfn_t pfn) { -if (pfn_valid (pfn)) -return !is_zero_pfn (pfn) &&PageReserved (pfn_to_page (pfn)) && -/* -* Some reserved pages, such as those from NVDIMM -* DAX devices, are not for MMIO, and can be mapped -* with cached memory type for better performance. -* However, the above check misconceives those pages -* as MMIO, and results in KVM mapping them with UC -* memory type, which would hurt the performance. -* Therefore, we check the host memory type in addition -* and only treat UC/UC-/WC pages as MMIO. -*/ - (!pat_enabled () ||pat_pfn_immune_to_uc_mtrr (pfn)); +struct page *page =pfn_to_online_page (pfn); + +/* +* Online pages consist of pages managed by the buddy. Especially, +* ZONE_DEVICE pages are never online. Online pages that are reserved +* indicate the zero page and MMIO pages. +*/ +if (page) +return !is_zero_pfn (pfn) &&PageReserved (pfn_to_page (pfn)); -return !e820__mapped_raw_any (pfn_to_hpa (pfn), -pfn_to_hpa (pfn +1 ) -1 , -E820_TYPE_RAM ); +/* +* Any RAM that is not online (e.g., mapped via /dev/mem without +* a memmap or with an uninitialized memmap) is not MMIO. +*/ +if (e820__mapped_raw_any (pfn_to_hpa (pfn),pfn_to_hpa (pfn +1 ) -1 , +E820_TYPE_RAM )) +return false; + +/* +* Finally, anything with a valid memmap could be ZONE_DEVICE - or the +* memmap could be uninitialized. Treat only UC/UC-/WC pages as MMIO. +*/ +return pfn_valid () && !pat_enabled () ||pat_pfn_immune_to_uc_mtrr (pfn); }/* Bits which may be returned by set_spte() */
Signature · 30 lines
2.21.0 And also virt/kvm/kvm_main.c:kvm_is_reserved_pfn() From 928be02b293750e6076c8622268ba41ecd8819e9 Mon Sep 17 00:00:00 2001 From: David Hildenbrand <[email protected]> Date: Tue, 22 Oct 2019 10:26:46 +0200 Subject: [PATCH v1] KVM:Prepare kvm_is_reserved_pfn() for PG_reserved changes Right now, ZONE_DEVICE memory is always set PG_reserved. We want to change that in the future. KVM has this weird use case that you can map anything from /dev/mem into the guest. pfn_valid() is not a reliable check whether the memmap was initialized and can be touched. pfn_to_online_page() makes sure that we have an initialized memmap. Note that ZONE_DEVICE memory is never online (IOW, managed by the buddy). Switching to pfn_to_online_page() keeps the existing behavior for PFNs without a memmap and for ZONE_DEVICE memory. They are treated as reserved and the page is not touched (e.g., to set it dirty or accessed). Signed-off-by: David Hildenbrand <[email protected]> --- virt/kvm/kvm_main.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
virt/kvm/kvm_main.c +8 -2
@@ -151,9 +151,15 @@ __weak int kvm_arch_mmu_notifier_invalidate_range(struct kvm *kvm,bool kvm_is_reserved_pfn (kvm_pfn_t pfn) { -if (pfn_valid (pfn)) -return PageReserved (pfn_to_page (pfn)); +struct page *page =pfn_to_online_page (pfn); +/* +* We treat any pages that are not online (not managed by the buddy) +* as reserved - this includes ZONE_DEVICE pages and pages without +* a memmap (e.g.., mapped via /dev/mem). +*/ +if (page) +return PageReserved (pfn_to_page (pfn));return true; }
Signature · 13 lines
2.21.0 I'd like to note that the pfn_valid() check in __kvm_map_gfn() is also bogus, but switching pfn_to_online_page() is not possible, as we would treat ZONE_DEVICE memory differently then. -- Thanks, David / dhildenb
Quoted 18 lines · David Hildenbrand
Quoted 17 lines · Michal Hocko
Quoted 16 lines · David Hildenbrand
Quoted 15 lines · Michal Hocko
Quoted 13 lines · David Hildenbrand
Quoted 9 lines · David Hildenbrand
Two cleanups that popped up while working on (and discussing) virtio-mem:
https://lkml.org/lkml/2019/9/19/463Tested with DIMMs on x86.
As discussed with michal in v1, I'll soon look into removing the use of
PG_reservedduring memory onlining completely - most probably disallowing to offline memory blocks with holes, cleaning up the onlining+offlining code.BTW, I remember that
ZONE_DEVICEpages are still required to be setPG_reserved. That has to be sorted out first.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Do they?
Yes, especially KVM code :/
Details please?
Oh, and I think you might be wondering "how can we have RAM without a memmap in the guest", see
https://lwn.net/Articles/778240/
Signature · 3 lines
Thanks, David / dhildenb
On Tue 22-10-19 10:32:11, David Hildenbrand wrote:
[...]
Quoted 1 line
E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()
Thanks for these references. I am not really familiar with kvm so I cannot really comment on the specific code but I am wondering why it simply doesn't check for ZONE_DEVICE explicitly? Also we do care about holes in RAM (from the early boot), those should be reserved already AFAIR. So we are left with hotplugged memory with holes and I am not really sure we should bother with this until there is a clear usecase in sight.
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 11 lines · Michal Hocko
On Tue 22-10-19 10:32:11, David Hildenbrand wrote:
[...]Quoted 1 line
E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()
Thanks for these references. I am not really familiar with kvm so I cannot really comment on the specific code but I am wondering why it simply doesn't check for
ZONE_DEVICEexplicitly? Also we do care about holes in RAM (from the early boot), those should be reserved already AFAIR. So we are left with hotplugged memory with holes and I am not really sure we should bother with this until there is a clear usecase in sight.
Well, checking for ZONE_DEVICE is only possible if you have an initialized memmap. And that is not guaranteed when you start mapping random stuff into your guest via /dev/mem.
I am reworking these patches right now and audit the whole kernel for PageReserved() checks that might affect ZONE_DEVICE. I'll send the collection of patches as RFC.
Signature · 3 lines
Thanks, David / dhildenb
Quoted 13 lines · David Hildenbrand
Quoted 11 lines · Michal Hocko
On Tue 22-10-19 10:32:11, David Hildenbrand wrote:
[...]Quoted 1 line
E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()
Thanks for these references. I am not really familiar with kvm so I cannot really comment on the specific code but I am wondering why it simply doesn't check for
ZONE_DEVICEexplicitly? Also we do care about holes in RAM (from the early boot), those should be reserved already AFAIR. So we are left with hotplugged memory with holes and I am not really sure we should bother with this until there is a clear usecase in sight.
Quoted 3 lines
Well, checking for
ZONE_DEVICEis only possible if you have an initialized memmap. And that is not guaranteed when you start mapping random stuff into your guest via/dev/mem.
Yes, I can understand that part but checking PageReserved on an uninitialized memmap is pointless as well. So if you can test for it you can very well test for ZONE_DEVICE as well. PageReserved -> ZONE_DEVICE is a terrible assumption.
Quoted 3 lines
I am reworking these patches right now and audit the whole kernel for
PageReserved()checks that might affectZONE_DEVICE. I'll send the collection of patches as RFC.
Thanks a lot!
Signature · 2 lines
Michal Hocko SUSE Labs
Quoted 14 lines · Michal Hocko
Quoted 13 lines · David Hildenbrand
Quoted 11 lines · Michal Hocko
On Tue 22-10-19 10:32:11, David Hildenbrand wrote:
[...]Quoted 1 line
E.g., arch/x86/kvm/mmu.c:kvm_is_mmio_pfn()
Thanks for these references. I am not really familiar with kvm so I cannot really comment on the specific code but I am wondering why it simply doesn't check for
ZONE_DEVICEexplicitly? Also we do care about holes in RAM (from the early boot), those should be reserved already AFAIR. So we are left with hotplugged memory with holes and I am not really sure we should bother with this until there is a clear usecase in sight.
Quoted 6 lines
Quoted 3 lines
Well, checking for
ZONE_DEVICEis only possible if you have an initialized memmap. And that is not guaranteed when you start mapping random stuff into your guest via/dev/mem.Yes, I can understand that part but checking PageReserved on an uninitialized memmap is pointless as well. So if you can test for it you
That's why I add pfn_to_online_page() :)
Quoted 2 lines
can very well test for
ZONE_DEVICEas well. PageReserved ->ZONE_DEVICEis a terrible assumption.
Indeed, it is. But there are more parts in the kernel that I'll be fixing. Stay tuned.
Signature · 3 lines
Thanks, David / dhildenb