| Message ID | 20220330122605.247613-5-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: s390: pv: implement lazy destroy for reboot | expand |
On 3/30/22 14:25, Claudio Imbrenda wrote: > Refactor s390_reset_acc so that it can be reused in upcoming patches. > > We don't want to hold all the locks used in a walk_page_range for too > long, and the destroy page UVC does take some time to complete. > Therefore we quickly gather the pages to destroy, and then destroy them > without holding all the locks. > > The new refactored function optionally allows to return early without > completing if a fatal signal is pending (and return and appropriate > error code). Two wrappers are provided to call the new function. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > (dropping Janosch's Ack because of major changes to the patch) Reviewed-by: Janosch Frank <frankja@linux.ibm.com> [...] > +#define DESTROY_LOOP_THRESHOLD 32 A question out of curiosity: Is there any particular reason for the number? Have you tested other numbers and experienced a speedup/slowdown? > + > +struct reset_walk_state { > + unsigned long next; > + unsigned long count; > + unsigned long pfns[DESTROY_LOOP_THRESHOLD]; > +}; > + > +static int s390_gather_pages(pte_t *ptep, unsigned long addr, > + unsigned long next, struct mm_walk *walk) > { > + struct reset_walk_state *p = walk->private; > pte_t pte = READ_ONCE(*ptep); > > - /* There is a reference through the mapping */ > - if (pte_present(pte)) > - WARN_ON_ONCE(uv_destroy_owned_page(pte_val(pte) & PAGE_MASK)); > - > - return 0; > + if (pte_present(pte)) { > + /* we have a reference from the mapping, take an extra one */ > + get_page(phys_to_page(pte_val(pte))); > + p->pfns[p->count] = phys_to_pfn(pte_val(pte)); > + p->next = next; > + p->count++; > + } > + return p->count >= DESTROY_LOOP_THRESHOLD; > } > > -static const struct mm_walk_ops reset_acc_walk_ops = { > - .pte_entry = __s390_reset_acc, > +static const struct mm_walk_ops gather_pages_ops = { > + .pte_entry = s390_gather_pages, > }; > > -#include <linux/sched/mm.h> > -void s390_reset_acc(struct mm_struct *mm) > +/* > + * Call the Destroy secure page UVC on each page in the given array of PFNs. > + * Each page needs to have an extra reference, which will be released here. > + */ > +void s390_uv_destroy_pfns(unsigned long count, unsigned long *pfns) > { > - if (!mm_is_protected(mm)) > - return; > - /* > - * we might be called during > - * reset: we walk the pages and clear > - * close of all kvm file descriptors: we walk the pages and clear > - * exit of process on fd closure: vma already gone, do nothing > - */ > - if (!mmget_not_zero(mm)) > - return; > - mmap_read_lock(mm); > - walk_page_range(mm, 0, TASK_SIZE, &reset_acc_walk_ops, NULL); > - mmap_read_unlock(mm); > - mmput(mm); > + unsigned long i; > + > + for (i = 0; i < count; i++) { > + /* we always have an extra reference */ > + uv_destroy_owned_page(pfn_to_phys(pfns[i])); > + /* get rid of the extra reference */ > + put_page(pfn_to_page(pfns[i])); > + cond_resched(); > + } > +} > +EXPORT_SYMBOL_GPL(s390_uv_destroy_pfns); > + > +/** > + * __s390_uv_destroy_range - Walk the given range of the given address > + * space, and call the destroy secure page UVC on each page. > + * Optionally exit early if a fatal signal is pending. > + * @mm the mm to operate on > + * @start the start of the range > + * @end the end of the range > + * @interruptible if not 0, stop when a fatal signal is received > + * Return: 0 on success, -EINTR if the function stopped before completing > + */ > +int __s390_uv_destroy_range(struct mm_struct *mm, unsigned long start, > + unsigned long end, bool interruptible) > +{ > + struct reset_walk_state state = { .next = start }; > + int r = 1; > + > + while (r > 0) { > + state.count = 0; > + mmap_read_lock(mm); > + r = walk_page_range(mm, state.next, end, &gather_pages_ops, &state); > + mmap_read_unlock(mm); > + cond_resched(); > + s390_uv_destroy_pfns(state.count, state.pfns); > + if (interruptible && fatal_signal_pending(current)) > + return -EINTR; > + } > + return 0; > } > -EXPORT_SYMBOL_GPL(s390_reset_acc); > +EXPORT_SYMBOL_GPL(__s390_uv_destroy_range); > > /** > * s390_remove_old_asce - Remove the topmost level of page tables from the
On Thu, 31 Mar 2022 15:25:31 +0200 Janosch Frank <frankja@linux.ibm.com> wrote: > On 3/30/22 14:25, Claudio Imbrenda wrote: > > Refactor s390_reset_acc so that it can be reused in upcoming patches. > > > > We don't want to hold all the locks used in a walk_page_range for too > > long, and the destroy page UVC does take some time to complete. > > Therefore we quickly gather the pages to destroy, and then destroy them > > without holding all the locks. > > > > The new refactored function optionally allows to return early without > > completing if a fatal signal is pending (and return and appropriate > > error code). Two wrappers are provided to call the new function. > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > (dropping Janosch's Ack because of major changes to the patch) > > Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > > [...] > > +#define DESTROY_LOOP_THRESHOLD 32 > > A question out of curiosity: > Is there any particular reason for the number? > Have you tested other numbers and experienced a speedup/slowdown? to be honest no, it just seemed a good tradeoff between size and callback overhead > > > + > > +struct reset_walk_state { > > + unsigned long next; > > + unsigned long count; > > + unsigned long pfns[DESTROY_LOOP_THRESHOLD]; > > +}; > > + > > +static int s390_gather_pages(pte_t *ptep, unsigned long addr, > > + unsigned long next, struct mm_walk *walk) > > { > > + struct reset_walk_state *p = walk->private; > > pte_t pte = READ_ONCE(*ptep); > > > > - /* There is a reference through the mapping */ > > - if (pte_present(pte)) > > - WARN_ON_ONCE(uv_destroy_owned_page(pte_val(pte) & PAGE_MASK)); > > - > > - return 0; > > + if (pte_present(pte)) { > > + /* we have a reference from the mapping, take an extra one */ > > + get_page(phys_to_page(pte_val(pte))); > > + p->pfns[p->count] = phys_to_pfn(pte_val(pte)); > > + p->next = next; > > + p->count++; > > + } > > + return p->count >= DESTROY_LOOP_THRESHOLD; > > } > > > > -static const struct mm_walk_ops reset_acc_walk_ops = { > > - .pte_entry = __s390_reset_acc, > > +static const struct mm_walk_ops gather_pages_ops = { > > + .pte_entry = s390_gather_pages, > > }; > > > > -#include <linux/sched/mm.h> > > -void s390_reset_acc(struct mm_struct *mm) > > +/* > > + * Call the Destroy secure page UVC on each page in the given array of PFNs. > > + * Each page needs to have an extra reference, which will be released here. > > + */ > > +void s390_uv_destroy_pfns(unsigned long count, unsigned long *pfns) > > { > > - if (!mm_is_protected(mm)) > > - return; > > - /* > > - * we might be called during > > - * reset: we walk the pages and clear > > - * close of all kvm file descriptors: we walk the pages and clear > > - * exit of process on fd closure: vma already gone, do nothing > > - */ > > - if (!mmget_not_zero(mm)) > > - return; > > - mmap_read_lock(mm); > > - walk_page_range(mm, 0, TASK_SIZE, &reset_acc_walk_ops, NULL); > > - mmap_read_unlock(mm); > > - mmput(mm); > > + unsigned long i; > > + > > + for (i = 0; i < count; i++) { > > + /* we always have an extra reference */ > > + uv_destroy_owned_page(pfn_to_phys(pfns[i])); > > + /* get rid of the extra reference */ > > + put_page(pfn_to_page(pfns[i])); > > + cond_resched(); > > + } > > +} > > +EXPORT_SYMBOL_GPL(s390_uv_destroy_pfns); > > + > > +/** > > + * __s390_uv_destroy_range - Walk the given range of the given address > > + * space, and call the destroy secure page UVC on each page. > > + * Optionally exit early if a fatal signal is pending. > > + * @mm the mm to operate on > > + * @start the start of the range > > + * @end the end of the range > > + * @interruptible if not 0, stop when a fatal signal is received > > + * Return: 0 on success, -EINTR if the function stopped before completing > > + */ > > +int __s390_uv_destroy_range(struct mm_struct *mm, unsigned long start, > > + unsigned long end, bool interruptible) > > +{ > > + struct reset_walk_state state = { .next = start }; > > + int r = 1; > > + > > + while (r > 0) { > > + state.count = 0; > > + mmap_read_lock(mm); > > + r = walk_page_range(mm, state.next, end, &gather_pages_ops, &state); > > + mmap_read_unlock(mm); > > + cond_resched(); > > + s390_uv_destroy_pfns(state.count, state.pfns); > > + if (interruptible && fatal_signal_pending(current)) > > + return -EINTR; > > + } > > + return 0; > > } > > -EXPORT_SYMBOL_GPL(s390_reset_acc); > > +EXPORT_SYMBOL_GPL(__s390_uv_destroy_range); > > > > /** > > * s390_remove_old_asce - Remove the topmost level of page tables from the >
diff --git a/arch/s390/include/asm/gmap.h b/arch/s390/include/asm/gmap.h index 746e18bf8984..0baaa127614b 100644 --- a/arch/s390/include/asm/gmap.h +++ b/arch/s390/include/asm/gmap.h @@ -147,7 +147,42 @@ 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); -void s390_reset_acc(struct mm_struct *mm); void s390_remove_old_asce(struct gmap *gmap); int s390_replace_asce(struct gmap *gmap); +void s390_uv_destroy_pfns(unsigned long count, unsigned long *pfns); +int __s390_uv_destroy_range(struct mm_struct *mm, unsigned long start, + unsigned long end, bool interruptible); + +/** + * s390_uv_destroy_range - Destroy a range of pages in the given mm. + * @mm the mm on which to operate on + * @start the start of the range + * @end the end of the range + * + * This function will call cond_sched, so it should not generate stalls, but + * it will otherwise only return when it completed. + */ +static inline void s390_uv_destroy_range(struct mm_struct *mm, unsigned long start, + unsigned long end) +{ + (void)__s390_uv_destroy_range(mm, start, end, false); +} + +/** + * s390_uv_destroy_range_interruptible - Destroy a range of pages in the + * given mm, but stop when a fatal signal is received. + * @mm the mm on which to operate on + * @start the start of the range + * @end the end of the range + * + * This function will call cond_sched, so it should not generate stalls. If + * a fatal signal is received, it will return with -EINTR immediately, + * without finishing destroying the whole range. Upon successful + * completion, 0 is returned. + */ +static inline int s390_uv_destroy_range_interruptible(struct mm_struct *mm, unsigned long start, + unsigned long end) +{ + return __s390_uv_destroy_range(mm, start, end, true); +} #endif /* _ASM_S390_GMAP_H */ diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index 3c59ef763dde..2ab22500e092 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -12,6 +12,8 @@ #include <asm/gmap.h> #include <asm/uv.h> #include <asm/mman.h> +#include <linux/pagewalk.h> +#include <linux/sched/mm.h> #include "kvm-s390.h" int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) @@ -157,8 +159,14 @@ int kvm_s390_pv_deinit_vm(struct kvm *kvm, u16 *rc, u16 *rrc) { int cc; - /* make all pages accessible before destroying the guest */ - s390_reset_acc(kvm->mm); + /* + * if the mm still has a mapping, make all its pages accessible + * before destroying the guest + */ + if (mmget_not_zero(kvm->mm)) { + s390_uv_destroy_range(kvm->mm, 0, TASK_SIZE); + mmput(kvm->mm); + } cc = uv_cmd_nodata(kvm_s390_pv_get_handle(kvm), UVC_CMD_DESTROY_SEC_CONF, rc, rrc); diff --git a/arch/s390/mm/gmap.c b/arch/s390/mm/gmap.c index 3b42bf7adb77..f6a80f139b84 100644 --- a/arch/s390/mm/gmap.c +++ b/arch/s390/mm/gmap.c @@ -2676,44 +2676,81 @@ void s390_reset_cmma(struct mm_struct *mm) } EXPORT_SYMBOL_GPL(s390_reset_cmma); -/* - * make inaccessible pages accessible again - */ -static int __s390_reset_acc(pte_t *ptep, unsigned long addr, - unsigned long next, struct mm_walk *walk) +#define DESTROY_LOOP_THRESHOLD 32 + +struct reset_walk_state { + unsigned long next; + unsigned long count; + unsigned long pfns[DESTROY_LOOP_THRESHOLD]; +}; + +static int s390_gather_pages(pte_t *ptep, unsigned long addr, + unsigned long next, struct mm_walk *walk) { + struct reset_walk_state *p = walk->private; pte_t pte = READ_ONCE(*ptep); - /* There is a reference through the mapping */ - if (pte_present(pte)) - WARN_ON_ONCE(uv_destroy_owned_page(pte_val(pte) & PAGE_MASK)); - - return 0; + if (pte_present(pte)) { + /* we have a reference from the mapping, take an extra one */ + get_page(phys_to_page(pte_val(pte))); + p->pfns[p->count] = phys_to_pfn(pte_val(pte)); + p->next = next; + p->count++; + } + return p->count >= DESTROY_LOOP_THRESHOLD; } -static const struct mm_walk_ops reset_acc_walk_ops = { - .pte_entry = __s390_reset_acc, +static const struct mm_walk_ops gather_pages_ops = { + .pte_entry = s390_gather_pages, }; -#include <linux/sched/mm.h> -void s390_reset_acc(struct mm_struct *mm) +/* + * Call the Destroy secure page UVC on each page in the given array of PFNs. + * Each page needs to have an extra reference, which will be released here. + */ +void s390_uv_destroy_pfns(unsigned long count, unsigned long *pfns) { - if (!mm_is_protected(mm)) - return; - /* - * we might be called during - * reset: we walk the pages and clear - * close of all kvm file descriptors: we walk the pages and clear - * exit of process on fd closure: vma already gone, do nothing - */ - if (!mmget_not_zero(mm)) - return; - mmap_read_lock(mm); - walk_page_range(mm, 0, TASK_SIZE, &reset_acc_walk_ops, NULL); - mmap_read_unlock(mm); - mmput(mm); + unsigned long i; + + for (i = 0; i < count; i++) { + /* we always have an extra reference */ + uv_destroy_owned_page(pfn_to_phys(pfns[i])); + /* get rid of the extra reference */ + put_page(pfn_to_page(pfns[i])); + cond_resched(); + } +} +EXPORT_SYMBOL_GPL(s390_uv_destroy_pfns); + +/** + * __s390_uv_destroy_range - Walk the given range of the given address + * space, and call the destroy secure page UVC on each page. + * Optionally exit early if a fatal signal is pending. + * @mm the mm to operate on + * @start the start of the range + * @end the end of the range + * @interruptible if not 0, stop when a fatal signal is received + * Return: 0 on success, -EINTR if the function stopped before completing + */ +int __s390_uv_destroy_range(struct mm_struct *mm, unsigned long start, + unsigned long end, bool interruptible) +{ + struct reset_walk_state state = { .next = start }; + int r = 1; + + while (r > 0) { + state.count = 0; + mmap_read_lock(mm); + r = walk_page_range(mm, state.next, end, &gather_pages_ops, &state); + mmap_read_unlock(mm); + cond_resched(); + s390_uv_destroy_pfns(state.count, state.pfns); + if (interruptible && fatal_signal_pending(current)) + return -EINTR; + } + return 0; } -EXPORT_SYMBOL_GPL(s390_reset_acc); +EXPORT_SYMBOL_GPL(__s390_uv_destroy_range); /** * s390_remove_old_asce - Remove the topmost level of page tables from the
Refactor s390_reset_acc so that it can be reused in upcoming patches. We don't want to hold all the locks used in a walk_page_range for too long, and the destroy page UVC does take some time to complete. Therefore we quickly gather the pages to destroy, and then destroy them without holding all the locks. The new refactored function optionally allows to return early without completing if a fatal signal is pending (and return and appropriate error code). Two wrappers are provided to call the new function. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> (dropping Janosch's Ack because of major changes to the patch) --- arch/s390/include/asm/gmap.h | 37 +++++++++++++- arch/s390/kvm/pv.c | 12 ++++- arch/s390/mm/gmap.c | 95 +++++++++++++++++++++++++----------- 3 files changed, 112 insertions(+), 32 deletions(-)