* Re: [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests
@ 2024-04-01 9:47 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2024-04-01 9:47 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240321215954.177730-3-david@redhat.com>
References: <20240321215954.177730-3-david@redhat.com>
TO: David Hildenbrand <david@redhat.com>
Hi David,
kernel test robot noticed the following build warnings:
[auto build test WARNING on kvms390/next]
[also build test WARNING on s390/features linus/master v6.9-rc2]
[cannot apply to akpm-mm/mm-everything next-20240328]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/David-Hildenbrand/mm-userfaultfd-don-t-place-zeropages-when-zeropages-are-disallowed/20240322-060251
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git next
patch link: https://lore.kernel.org/r/20240321215954.177730-3-david%40redhat.com
patch subject: [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests
:::::: branch date: 11 days ago
:::::: commit date: 11 days ago
config: s390-randconfig-r071-20240325 (https://download.01.org/0day-ci/archive/20240401/202404011730.2TEIocud-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 23de3862dce582ce91c1aa914467d982cb1a73b4)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202404011730.2TEIocud-lkp@intel.com/
smatch warnings:
arch/s390/mm/gmap.c:2656 __s390_unshare_zeropages() error: uninitialized symbol 'rc'.
vim +/rc +2656 arch/s390/mm/gmap.c
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2598
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2599 /*
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2600 * Unshare all shared zeropages, replacing them by anonymous pages. Note that
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2601 * we cannot simply zap all shared zeropages, because this could later
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2602 * trigger unexpected userfaultfd missing events.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2603 *
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2604 * This must be called after mm->context.allow_cow_sharing was
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2605 * set to 0, to avoid future mappings of shared zeropages.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2606 *
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2607 * mm contracts with s390, that even if mm were to remove a page table,
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2608 * and racing with walk_page_range_vma() calling pte_offset_map_lock()
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2609 * would fail, it will never insert a page table containing empty zero
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2610 * pages once mm_forbids_zeropage(mm) i.e.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2611 * mm->context.allow_cow_sharing is set to 0.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2612 */
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2613 static int __s390_unshare_zeropages(struct mm_struct *mm)
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2614 {
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2615 struct vm_area_struct *vma;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2616 VMA_ITERATOR(vmi, mm, 0);
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2617 unsigned long addr;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2618 int rc;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2619
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2620 for_each_vma(vmi, vma) {
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2621 /*
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2622 * We could only look at COW mappings, but it's more future
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2623 * proof to catch unexpected zeropages in other mappings and
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2624 * fail.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2625 */
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2626 if ((vma->vm_flags & VM_PFNMAP) || is_vm_hugetlb_page(vma))
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2627 continue;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2628 addr = vma->vm_start;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2629
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2630 retry:
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2631 rc = walk_page_range_vma(vma, addr, vma->vm_end,
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2632 &find_zeropage_ops, &addr);
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2633 if (rc <= 0)
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2634 continue;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2635
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2636 /* addr was updated by find_zeropage_pte_entry() */
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2637 rc = handle_mm_fault(vma, addr,
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2638 FAULT_FLAG_UNSHARE | FAULT_FLAG_REMOTE,
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2639 NULL);
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2640 if (rc & VM_FAULT_OOM)
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2641 return -ENOMEM;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2642 /*
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2643 * See break_ksm(): even after handle_mm_fault() returned 0, we
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2644 * must start the lookup from the current address, because
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2645 * handle_mm_fault() may back out if there's any difficulty.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2646 *
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2647 * VM_FAULT_SIGBUS and VM_FAULT_SIGSEGV are unexpected but
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2648 * maybe they could trigger in the future on concurrent
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2649 * truncation. In that case, the shared zeropage would be gone
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2650 * and we can simply retry and make progress.
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2651 */
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2652 cond_resched();
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2653 goto retry;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2654 }
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2655
9a7a43a7ce9759 David Hildenbrand 2024-03-21 @2656 return rc;
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2657 }
9a7a43a7ce9759 David Hildenbrand 2024-03-21 2658
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v1 0/2] s390/mm: shared zeropage + KVM fix and optimization
@ 2024-03-21 21:59 David Hildenbrand
2024-03-21 21:59 ` [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests David Hildenbrand
0 siblings, 1 reply; 4+ messages in thread
From: David Hildenbrand @ 2024-03-21 21:59 UTC (permalink / raw)
To: linux-kernel
Cc: linux-mm, David Hildenbrand, Christian Borntraeger, Janosch Frank,
Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Andrew Morton,
Peter Xu, Alexander Gordeev, Sven Schnelle, Gerald Schaefer,
Andrea Arcangeli, kvm, linux-s390
This series fixes one issue with uffd + shared zeropages on s390x and
optimizes "ordinary" KVM guests to make use of shared zeropages again.
userfaultfd could currently end up mapping shared zeropages into processes
that forbid shared zeropages. This only apples to s390x, relevant for
handling PV guests and guests that use storage kets correctly. Fix it
by placing a zeroed folio instead of the shared zeropage during
UFFDIO_ZEROPAGE instead.
I stumbled over this issue while looking into a customer scenario that
is using:
(1) Memory ballooning for dynamic resizing. Start a VM with, say, 100 GiB
and inflate the balloon during boot to 60 GiB. The VM has ~40 GiB
available and additional memory can be "fake hotplugged" to the VM
later on demand by deflating the balloon. Actual memory overcommit is
not desired, so physical memory would only be moved between VMs.
(2) Live migration of VMs between sites to evacuate servers in case of
emergency.
Without the shared zeropage, during (2), the VM would suddenly consume
100 GiB on the migration source and destination. On the migration source,
where we don't excpect memory overcommit, we could easilt end up crashing
the VM during migration.
Independent of that, memory handed back to the hypervisor using "free page
reporting" would end up consuming actual memory after the migration on the
destination, not getting freed up until reused+freed again.
While there might be ways to optimize parts of this in QEMU, we really
should just support the shared zeropage again for ordinary VMs.
We only expect legcy guests to make use of storage keys, so let's handle
zeropages again when enabling storage keys or when enabling PV. To not
break userfaultfd like we did in the past, don't zap the shared zeropages,
but instead trigger unsharing faults, just like we do for unsharing
KSM pages in break_ksm().
Unsharing faults will simply replace the shared zeropage by a zeroed
anonymous folio. We can already trigger the same fault path using GUP,
when trying to long-term pin a shared zeropage, but also when unmerging
a KSM-placed zeropages, so this is nothing new.
Patch #1 tested on 86-64 by forcing mm_forbids_zeropage() to be 1, and
running the uffd selftests.
Patch #2 tested on s390x: the live migration scenario now works as
expected, and kvm-unit-tests that trigger usage of skeys work well, whereby
I can see detection and unsharing of shared zeropages.
Based on current mm-unstable. Maybe at least the second patch should
go via the s390x tree, I think patch #1 could go that route as well.
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Janosch Frank <frankja@linux.ibm.com>
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-s390@vger.kernel.org
David Hildenbrand (2):
mm/userfaultfd: don't place zeropages when zeropages are disallowed
s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests
arch/s390/include/asm/gmap.h | 2 +-
arch/s390/include/asm/mmu.h | 5 +
arch/s390/include/asm/mmu_context.h | 1 +
arch/s390/include/asm/pgtable.h | 15 ++-
arch/s390/kvm/kvm-s390.c | 4 +-
arch/s390/mm/gmap.c | 163 +++++++++++++++++++++-------
mm/userfaultfd.c | 35 ++++++
7 files changed, 178 insertions(+), 47 deletions(-)
--
2.43.2
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests 2024-03-21 21:59 [PATCH v1 0/2] s390/mm: shared zeropage + KVM fix and optimization David Hildenbrand @ 2024-03-21 21:59 ` David Hildenbrand 2024-03-22 10:22 ` Christian Borntraeger 0 siblings, 1 reply; 4+ messages in thread From: David Hildenbrand @ 2024-03-21 21:59 UTC (permalink / raw) To: linux-kernel Cc: linux-mm, David Hildenbrand, Christian Borntraeger, Janosch Frank, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Andrew Morton, Peter Xu, Alexander Gordeev, Sven Schnelle, Gerald Schaefer, Andrea Arcangeli, kvm, linux-s390 commit fa41ba0d08de ("s390/mm: avoid empty zero pages for KVM guests to avoid postcopy hangs") introduced an undesired side effect when combined with memory ballooning and VM migration: memory part of the inflated memory balloon will consume memory. Assuming we have a 100GiB VM and inflated the balloon to 40GiB. Our VM will consume ~60GiB of memory. If we now trigger a VM migration, hypervisors like QEMU will read all VM memory. As s390x does not support the shared zeropage, we'll end up allocating for all previously-inflated memory part of the memory balloon: 50 GiB. So we might easily (unexpectedly) crash the VM on the migration source. Even worse, hypervisors like QEMU optimize for zeropage migration to not consume memory on the migration destination: when migrating a "page full of zeroes", on the migration destination they check whether the target memory is already zero (by reading the destination memory) and avoid writing to the memory to not allocate memory: however, s390x will also allocate memory here, implying that also on the migration destination, we will end up allocating all previously-inflated memory part of the memory balloon. This is especially bad if actual memory overcommit was not desired, when memory ballooning is used for dynamic VM memory resizing, setting aside some memory during boot that can be added later on demand. Alternatives like virtio-mem that would avoid this issue are not yet available on s390x. There could be ways to optimize some cases in user space: before reading memory in an anonymous private mapping on the migration source, check via /proc/self/pagemap if anything is already populated. Similarly check on the migration destination before reading. While that would avoid populating tables full of shared zeropages on all architectures, it's harder to get right and performant, and requires user space changes. Further, with posctopy live migration we must place a page, so there, "avoid touching memory to avoid allocating memory" is not really possible. (Note that a previously we would have falsely inserted shared zeropages into processes using UFFDIO_ZEROPAGE where mm_forbids_zeropage() would have actually forbidden it) PV is currently incompatible with memory ballooning, and in the common case, KVM guests don't make use of storage keys. Instead of zapping zeropages when enabling storage keys / PV, that turned out to be problematic in the past, let's do exactly the same we do with KSM pages: trigger unsharing faults to replace the shared zeropages by proper anonymous folios. What about added latency when enabling storage kes? Having a lot of zeropages in applicable environments (PV, legacy guests, unittests) is unexpected. Further, KSM could today already unshare the zeropages and unmerging KSM pages when enabling storage kets would unshare the KSM-placed zeropages in the same way, resulting in the same latency. Signed-off-by: David Hildenbrand <david@redhat.com> --- arch/s390/include/asm/gmap.h | 2 +- arch/s390/include/asm/mmu.h | 5 + arch/s390/include/asm/mmu_context.h | 1 + arch/s390/include/asm/pgtable.h | 15 ++- arch/s390/kvm/kvm-s390.c | 4 +- arch/s390/mm/gmap.c | 163 +++++++++++++++++++++------- 6 files changed, 143 insertions(+), 47 deletions(-) diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h index 5cc46e0dde620..9725586f42597 100644 --- a/arch/s390/include/asm/gmap.h +++ b/arch/s390/include/asm/gmap.h @@ -146,7 +146,7 @@ int gmap_mprotect_notify(struct gmap *, unsigned long start, void gmap_sync_dirty_log_pmd(struct gmap *gmap, unsigned long dirty_bitmap[4], unsigned long gaddr, unsigned long vmaddr); -int gmap_mark_unmergeable(void); +int s390_disable_cow_sharing(void); void s390_unlist_old_asce(struct gmap *gmap); int s390_replace_asce(struct gmap *gmap); void s390_uv_destroy_pfns(unsigned long count, unsigned long *pfns); diff --git a/arch/s390/include/asm/mmu.h b/arch/s390/include/asm/mmu.h index bb1b4bef1878b..4c2dc7abc2858 100644 --- a/arch/s390/include/asm/mmu.h +++ b/arch/s390/include/asm/mmu.h @@ -32,6 +32,11 @@ typedef struct { unsigned int uses_skeys:1; /* The mmu context uses CMM. */ unsigned int uses_cmm:1; + /* + * The mmu context allows COW-sharing of memory pages (KSM, zeropage). + * Note that COW-sharing during fork() is currently always allowed. + */ + unsigned int allow_cow_sharing:1; /* The gmaps associated with this context are allowed to use huge pages. */ unsigned int allow_gmap_hpage_1m:1; } mm_context_t; diff --git a/arch/s390/include/asm/mmu_context.h b/arch/s390/include/asm/mmu_context.h index 929af18b09081..a7789a9f62186 100644 --- a/arch/s390/include/asm/mmu_context.h +++ b/arch/s390/include/asm/mmu_context.h @@ -35,6 +35,7 @@ static inline int init_new_context(struct task_struct *tsk, mm->context.has_pgste = 0; mm->context.uses_skeys = 0; mm->context.uses_cmm = 0; + mm->context.allow_cow_sharing = 1; mm->context.allow_gmap_hpage_1m = 0; #endif switch (mm->context.asce_limit) { diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h index 60950e7a25f58..1a71cb19c0891 100644 --- a/arch/s390/include/asm/pgtable.h +++ b/arch/s390/include/asm/pgtable.h @@ -566,10 +566,19 @@ static inline pud_t set_pud_bit(pud_t pud, pgprot_t prot) } /* - * In the case that a guest uses storage keys - * faults should no longer be backed by zero pages + * As soon as the guest uses storage keys or enables PV, we deduplicate all + * mapped shared zeropages and prevent new shared zeropages from getting + * mapped. */ -#define mm_forbids_zeropage mm_has_pgste +static inline int mm_forbids_zeropage(struct mm_struct *mm) +{ +#ifdef CONFIG_PGSTE + if (!mm->context.allow_cow_sharing) + return 1; +#endif + return 0; +} + static inline int mm_uses_skeys(struct mm_struct *mm) { #ifdef CONFIG_PGSTE diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 5147b943a864a..db3392f0be212 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -2631,9 +2631,7 @@ static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd) if (r) break; - mmap_write_lock(current->mm); - r = gmap_mark_unmergeable(); - mmap_write_unlock(current->mm); + r = s390_disable_cow_sharing(); if (r) break; diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index 094b43b121cd5..9233b0acac895 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -2549,41 +2549,6 @@ static inline void thp_split_mm(struct mm_struct *mm) } #endif /* CONFIG_TRANSPARENT_HUGEPAGE */ -/* - * Remove all empty zero pages from the mapping for lazy refaulting - * - This must be called after mm->context.has_pgste is set, to avoid - * future creation of zero pages - * - This must be called after THP was disabled. - * - * mm contracts with s390, that even if mm were to remove a page table, - * racing with the loop below and so causing pte_offset_map_lock() to fail, - * it will never insert a page table containing empty zero pages once - * mm_forbids_zeropage(mm) i.e. mm->context.has_pgste is set. - */ -static int __zap_zero_pages(pmd_t *pmd, unsigned long start, - unsigned long end, struct mm_walk *walk) -{ - unsigned long addr; - - for (addr = start; addr != end; addr += PAGE_SIZE) { - pte_t *ptep; - spinlock_t *ptl; - - ptep = pte_offset_map_lock(walk->mm, pmd, addr, &ptl); - if (!ptep) - break; - if (is_zero_pfn(pte_pfn(*ptep))) - ptep_xchg_direct(walk->mm, addr, ptep, __pte(_PAGE_INVALID)); - pte_unmap_unlock(ptep, ptl); - } - return 0; -} - -static const struct mm_walk_ops zap_zero_walk_ops = { - .pmd_entry = __zap_zero_pages, - .walk_lock = PGWALK_WRLOCK, -}; - /* * switch on pgstes for its userspace process (for kvm) */ @@ -2601,22 +2566,140 @@ int s390_enable_sie(void) mm->context.has_pgste = 1; /* split thp mappings and disable thp for future mappings */ thp_split_mm(mm); - walk_page_range(mm, 0, TASK_SIZE, &zap_zero_walk_ops, NULL); mmap_write_unlock(mm); return 0; } EXPORT_SYMBOL_GPL(s390_enable_sie); -int gmap_mark_unmergeable(void) +static int find_zeropage_pte_entry(pte_t *pte, unsigned long addr, + unsigned long end, struct mm_walk *walk) +{ + unsigned long *found_addr = walk->private; + + /* Return 1 of the page is a zeropage. */ + if (is_zero_pfn(pte_pfn(*pte))) { + + /* + * Shared zeropage in e.g., a FS DAX mapping? We cannot do the + * right thing and likely don't care: FAULT_FLAG_UNSHARE + * currently only works in COW mappings, which is also where + * mm_forbids_zeropage() is checked. + */ + if (!is_cow_mapping(walk->vma->vm_flags)) + return -EFAULT; + + *found_addr = addr; + return 1; + } + return 0; +} + +static const struct mm_walk_ops find_zeropage_ops = { + .pte_entry = find_zeropage_pte_entry, + .walk_lock = PGWALK_WRLOCK, +}; + +/* + * Unshare all shared zeropages, replacing them by anonymous pages. Note that + * we cannot simply zap all shared zeropages, because this could later + * trigger unexpected userfaultfd missing events. + * + * This must be called after mm->context.allow_cow_sharing was + * set to 0, to avoid future mappings of shared zeropages. + * + * mm contracts with s390, that even if mm were to remove a page table, + * and racing with walk_page_range_vma() calling pte_offset_map_lock() + * would fail, it will never insert a page table containing empty zero + * pages once mm_forbids_zeropage(mm) i.e. + * mm->context.allow_cow_sharing is set to 0. + */ +static int __s390_unshare_zeropages(struct mm_struct *mm) +{ + struct vm_area_struct *vma; + VMA_ITERATOR(vmi, mm, 0); + unsigned long addr; + int rc; + + for_each_vma(vmi, vma) { + /* + * We could only look at COW mappings, but it's more future + * proof to catch unexpected zeropages in other mappings and + * fail. + */ + if ((vma->vm_flags & VM_PFNMAP) || is_vm_hugetlb_page(vma)) + continue; + addr = vma->vm_start; + +retry: + rc = walk_page_range_vma(vma, addr, vma->vm_end, + &find_zeropage_ops, &addr); + if (rc <= 0) + continue; + + /* addr was updated by find_zeropage_pte_entry() */ + rc = handle_mm_fault(vma, addr, + FAULT_FLAG_UNSHARE | FAULT_FLAG_REMOTE, + NULL); + if (rc & VM_FAULT_OOM) + return -ENOMEM; + /* + * See break_ksm(): even after handle_mm_fault() returned 0, we + * must start the lookup from the current address, because + * handle_mm_fault() may back out if there's any difficulty. + * + * VM_FAULT_SIGBUS and VM_FAULT_SIGSEGV are unexpected but + * maybe they could trigger in the future on concurrent + * truncation. In that case, the shared zeropage would be gone + * and we can simply retry and make progress. + */ + cond_resched(); + goto retry; + } + + return rc; +} + +static int __s390_disable_cow_sharing(struct mm_struct *mm) { + int rc; + + if (!mm->context.allow_cow_sharing) + return 0; + + mm->context.allow_cow_sharing = 0; + + /* Replace all shared zeropages by anonymous pages. */ + rc = __s390_unshare_zeropages(mm); /* * Make sure to disable KSM (if enabled for the whole process or * individual VMAs). Note that nothing currently hinders user space * from re-enabling it. */ - return ksm_disable(current->mm); + if (!rc) + rc = ksm_disable(mm); + if (rc) + mm->context.allow_cow_sharing = 1; + return rc; +} + +/* + * Disable most COW-sharing of memory pages for the whole process: + * (1) Disable KSM and unmerge/unshare any KSM pages. + * (2) Disallow shared zeropages and unshare any zerpages that are mapped. + * + * Not that we currently don't bother with COW-shared pages that are shared + * with parent/child processes due to fork(). + */ +int s390_disable_cow_sharing(void) +{ + int rc; + + mmap_write_lock(current->mm); + rc = __s390_disable_cow_sharing(current->mm); + mmap_write_unlock(current->mm); + return rc; } -EXPORT_SYMBOL_GPL(gmap_mark_unmergeable); +EXPORT_SYMBOL_GPL(s390_disable_cow_sharing); /* * Enable storage key handling from now on and initialize the storage @@ -2685,7 +2768,7 @@ int s390_enable_skey(void) goto out_up; mm->context.uses_skeys = 1; - rc = gmap_mark_unmergeable(); + rc = __s390_disable_cow_sharing(mm); if (rc) { mm->context.uses_skeys = 0; goto out_up; -- 2.43.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests 2024-03-21 21:59 ` [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests David Hildenbrand @ 2024-03-22 10:22 ` Christian Borntraeger 2024-03-22 17:08 ` David Hildenbrand 0 siblings, 1 reply; 4+ messages in thread From: Christian Borntraeger @ 2024-03-22 10:22 UTC (permalink / raw) To: David Hildenbrand, linux-kernel Cc: linux-mm, Janosch Frank, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Andrew Morton, Peter Xu, Alexander Gordeev, Sven Schnelle, Gerald Schaefer, Andrea Arcangeli, kvm, linux-s390 Am 21.03.24 um 22:59 schrieb David Hildenbrand: > commit fa41ba0d08de ("s390/mm: avoid empty zero pages for KVM guests to > avoid postcopy hangs") introduced an undesired side effect when combined > with memory ballooning and VM migration: memory part of the inflated > memory balloon will consume memory. > > Assuming we have a 100GiB VM and inflated the balloon to 40GiB. Our VM > will consume ~60GiB of memory. If we now trigger a VM migration, > hypervisors like QEMU will read all VM memory. As s390x does not support > the shared zeropage, we'll end up allocating for all previously-inflated > memory part of the memory balloon: 50 GiB. So we might easily > (unexpectedly) crash the VM on the migration source. > > Even worse, hypervisors like QEMU optimize for zeropage migration to not > consume memory on the migration destination: when migrating a > "page full of zeroes", on the migration destination they check whether the > target memory is already zero (by reading the destination memory) and avoid > writing to the memory to not allocate memory: however, s390x will also > allocate memory here, implying that also on the migration destination, we > will end up allocating all previously-inflated memory part of the memory > balloon. > > This is especially bad if actual memory overcommit was not desired, when > memory ballooning is used for dynamic VM memory resizing, setting aside > some memory during boot that can be added later on demand. Alternatives > like virtio-mem that would avoid this issue are not yet available on > s390x. > > There could be ways to optimize some cases in user space: before reading > memory in an anonymous private mapping on the migration source, check via > /proc/self/pagemap if anything is already populated. Similarly check on > the migration destination before reading. While that would avoid > populating tables full of shared zeropages on all architectures, it's > harder to get right and performant, and requires user space changes. > > Further, with posctopy live migration we must place a page, so there, > "avoid touching memory to avoid allocating memory" is not really > possible. (Note that a previously we would have falsely inserted > shared zeropages into processes using UFFDIO_ZEROPAGE where > mm_forbids_zeropage() would have actually forbidden it) > > PV is currently incompatible with memory ballooning, and in the common > case, KVM guests don't make use of storage keys. Instead of zapping > zeropages when enabling storage keys / PV, that turned out to be > problematic in the past, let's do exactly the same we do with KSM pages: > trigger unsharing faults to replace the shared zeropages by proper > anonymous folios. > > What about added latency when enabling storage kes? Having a lot of > zeropages in applicable environments (PV, legacy guests, unittests) is > unexpected. Further, KSM could today already unshare the zeropages > and unmerging KSM pages when enabling storage kets would unshare the > KSM-placed zeropages in the same way, resulting in the same latency. > > Signed-off-by: David Hildenbrand <david@redhat.com> Nice work. Looks good to me and indeed it fixes the memory over-consumption that you mentioned. Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com> (can also be seen with virsh managedsave; virsh start) I guess its too invasive for stable, but I would say it is real fix. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests 2024-03-22 10:22 ` Christian Borntraeger @ 2024-03-22 17:08 ` David Hildenbrand 0 siblings, 0 replies; 4+ messages in thread From: David Hildenbrand @ 2024-03-22 17:08 UTC (permalink / raw) To: Christian Borntraeger, linux-kernel Cc: linux-mm, Janosch Frank, Claudio Imbrenda, Heiko Carstens, Vasily Gorbik, Andrew Morton, Peter Xu, Alexander Gordeev, Sven Schnelle, Gerald Schaefer, Andrea Arcangeli, kvm, linux-s390 On 22.03.24 11:22, Christian Borntraeger wrote: > > > Am 21.03.24 um 22:59 schrieb David Hildenbrand: >> commit fa41ba0d08de ("s390/mm: avoid empty zero pages for KVM guests to >> avoid postcopy hangs") introduced an undesired side effect when combined >> with memory ballooning and VM migration: memory part of the inflated >> memory balloon will consume memory. >> >> Assuming we have a 100GiB VM and inflated the balloon to 40GiB. Our VM >> will consume ~60GiB of memory. If we now trigger a VM migration, >> hypervisors like QEMU will read all VM memory. As s390x does not support >> the shared zeropage, we'll end up allocating for all previously-inflated >> memory part of the memory balloon: 50 GiB. So we might easily >> (unexpectedly) crash the VM on the migration source. >> >> Even worse, hypervisors like QEMU optimize for zeropage migration to not >> consume memory on the migration destination: when migrating a >> "page full of zeroes", on the migration destination they check whether the >> target memory is already zero (by reading the destination memory) and avoid >> writing to the memory to not allocate memory: however, s390x will also >> allocate memory here, implying that also on the migration destination, we >> will end up allocating all previously-inflated memory part of the memory >> balloon. >> >> This is especially bad if actual memory overcommit was not desired, when >> memory ballooning is used for dynamic VM memory resizing, setting aside >> some memory during boot that can be added later on demand. Alternatives >> like virtio-mem that would avoid this issue are not yet available on >> s390x. >> >> There could be ways to optimize some cases in user space: before reading >> memory in an anonymous private mapping on the migration source, check via >> /proc/self/pagemap if anything is already populated. Similarly check on >> the migration destination before reading. While that would avoid >> populating tables full of shared zeropages on all architectures, it's >> harder to get right and performant, and requires user space changes. >> >> Further, with posctopy live migration we must place a page, so there, >> "avoid touching memory to avoid allocating memory" is not really >> possible. (Note that a previously we would have falsely inserted >> shared zeropages into processes using UFFDIO_ZEROPAGE where >> mm_forbids_zeropage() would have actually forbidden it) >> >> PV is currently incompatible with memory ballooning, and in the common >> case, KVM guests don't make use of storage keys. Instead of zapping >> zeropages when enabling storage keys / PV, that turned out to be >> problematic in the past, let's do exactly the same we do with KSM pages: >> trigger unsharing faults to replace the shared zeropages by proper >> anonymous folios. >> >> What about added latency when enabling storage kes? Having a lot of >> zeropages in applicable environments (PV, legacy guests, unittests) is >> unexpected. Further, KSM could today already unshare the zeropages >> and unmerging KSM pages when enabling storage kets would unshare the >> KSM-placed zeropages in the same way, resulting in the same latency. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > Nice work. Looks good to me and indeed it fixes the memory > over-consumption that you mentioned. Thanks for the very fast review and test! > > Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com> > Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com> > (can also be seen with virsh managedsave; virsh start) > > I guess its too invasive for stable, but I would say it is real fix. Should we add a Fixes: Tag? I refrained from doing so, treating this more like an optimization to restore the intended behavior at least as long as the VM does not use storage keys. -- Cheers, David / dhildenb ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-01 9:47 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-01 9:47 [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests kernel test robot -- strict thread matches above, loose matches on Subject: below -- 2024-03-21 21:59 [PATCH v1 0/2] s390/mm: shared zeropage + KVM fix and optimization David Hildenbrand 2024-03-21 21:59 ` [PATCH v1 2/2] s390/mm: re-enable the shared zeropage for !PV and !skeys KVM guests David Hildenbrand 2024-03-22 10:22 ` Christian Borntraeger 2024-03-22 17:08 ` David Hildenbrand
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.