Re: [PATCH mm-unstable v2 04/10] kvm/arm64: make stage2 page tables RCU safe
From: Yu Zhao <hidden>
Date: 2023-05-27 20:13:56
Also in:
kvm, kvmarm, linux-arm-kernel, linux-doc, linux-mm, linux-trace-kernel, lkml
On Sat, May 27, 2023 at 12:08 PM Oliver Upton [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Yu, On Fri, May 26, 2023 at 05:44:29PM -0600, Yu Zhao wrote:quoted
Stage2 page tables are currently not RCU safe against unmapping or VM destruction. The previous mmu_notifier_ops members rely on kvm->mmu_lock to synchronize with those operations. However, the new mmu_notifier_ops member test_clear_young() provides a fast path that does not take kvm->mmu_lock. To implement kvm_arch_test_clear_young() for that path, unmapped page tables need to be freed by RCU and kvm_free_stage2_pgd() needs to be after mmu_notifier_unregister(). Remapping, specifically stage2_free_removed_table(), is already RCU safe. Signed-off-by: Yu Zhao <redacted> --- arch/arm64/include/asm/kvm_pgtable.h | 2 ++ arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/hyp/pgtable.c | 8 ++++++-- arch/arm64/kvm/mmu.c | 17 ++++++++++++++++- 4 files changed, 25 insertions(+), 3 deletions(-)diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h index ff520598b62c..5cab52e3a35f 100644 --- a/arch/arm64/include/asm/kvm_pgtable.h +++ b/arch/arm64/include/asm/kvm_pgtable.h@@ -153,6 +153,7 @@ static inline bool kvm_level_supports_block_mapping(u32 level) * @put_page: Decrement the refcount on a page. When the * refcount reaches 0 the page is automatically * freed. + * @put_page_rcu: RCU variant of the above.You don't need to add yet another hook to implement this. I was working on lock-free walks in a separate context and arrived at the following: commit f82d264a37745e07ee28e116c336f139f681fd7f Author: Oliver Upton [off-list ref] Date: Mon May 1 08:53:37 2023 +0000 KVM: arm64: Consistently use free_removed_table() for stage-2 free_removed_table() is essential to the RCU-protected parallel walking scheme, as behind the scenes the cleanup is deferred until an RCU grace period. Nonetheless, the stage-2 unmap path calls put_page() directly, which leads to table memory being freed inline with the table walk. This is safe for the time being, as the stage-2 unmap walker is called while holding the write lock. A future change to KVM will further relax the locking mechanics around the stage-2 page tables to allow lock-free walkers protected only by RCU. As such, switch to the RCU-safe mechanism for freeing table memory. Signed-off-by: Oliver Upton [off-list ref]diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c index 3d61bd3e591d..bfbebdcb4ef0 100644 --- a/arch/arm64/kvm/hyp/pgtable.c +++ b/arch/arm64/kvm/hyp/pgtable.c@@ -1019,7 +1019,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx, kvm_granule_size(ctx->level)); if (childp) - mm_ops->put_page(childp); + mm_ops->free_removed_table(childp, ctx->level);
Thanks, Oliver. A couple of things I haven't had the chance to verify -- I'm hoping you could help clarify: 1. For unmapping, with free_removed_table(), wouldn't we have to look into the table we know it's empty unnecessarily? 2. For remapping and unmapping, how does free_removed_table() put the final refcnt on the table passed in? (Previously we had put_page(childp) in stage2_map_walk_table_post(). So I'm assuming we'd have to do something equivalent with free_removed_table().)