From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D1CD3C4345F for ; Fri, 19 Apr 2024 20:42:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4454E6B0083; Fri, 19 Apr 2024 16:42:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F5DC6B0085; Fri, 19 Apr 2024 16:42:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 26EC56B0087; Fri, 19 Apr 2024 16:42:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 07CF06B0083 for ; Fri, 19 Apr 2024 16:42:32 -0400 (EDT) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 7372581561 for ; Fri, 19 Apr 2024 20:42:31 +0000 (UTC) X-FDA: 82027454502.16.2370E99 Received: from mail-il1-f173.google.com (mail-il1-f173.google.com [209.85.166.173]) by imf11.hostedemail.com (Postfix) with ESMTP id A9CF140004 for ; Fri, 19 Apr 2024 20:42:29 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=HpSChfqH; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713559349; a=rsa-sha256; cv=none; b=oYpY42wiZSG2a1FBOxPRhr1jqH+ToE8S2V5UaYHo8OKQbr4CAdQVNtIiBI7gF7o2SIm9ni 7bjEF/5um4mQW2b3nQ2TbZqqiqFjKdO/23fK6nfdHikErbvgJOTgabn++l0l8QesWgPXUN 9q77kHZpZj05RKcz0MQPXhdsENHagtA= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=HpSChfqH; spf=pass (imf11.hostedemail.com: domain of jthoughton@google.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=jthoughton@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713559349; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=YrZKOhtpV6MTNw07l3xNfXkFWnpKfDtuHjOgNMG4wsw=; b=371prLgIl9KoLoEVSDvLfuEciPEmczyrP4iZGlpC+DTkz/1l2M34EoOfJi42nB9bhj5s83 VQIzk30VM6/bIpduMmCkoNx9l1NbZK0ooIgtwlbr4vtduXt9y+RLKZ+MMVkWjuerO8gBPs x2whuuaVhU4V6G48OUIdsEad1LLGT2o= Received: by mail-il1-f173.google.com with SMTP id e9e14a558f8ab-36b34effb04so7325ab.1 for ; Fri, 19 Apr 2024 13:42:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713559349; x=1714164149; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YrZKOhtpV6MTNw07l3xNfXkFWnpKfDtuHjOgNMG4wsw=; b=HpSChfqHPcNX9W4Plk3CnmDbEmlPvPLGygOD+IK91djNct+wrvYUuOCxrOiBKORhrH dtT+1rWFW8P04UsragJqmEorLWbmf7XpoxA3SiiKx9Blu9hQSSCZT2ylA6KiJAIgk2lt 2Mrre+aKKRVstJwW90hIWzoe5qr24Kd9OADP1tvJCrY9JGJCtM1CG91JvbDAs0/aXeIA CEafLA2PKzQRAiJXePuKGoNgYWMN3YITHyIj/Ck4mYF0uJeEBw7p15e65IV83TRHuRNc DkKFQ5fEjrvTbBaGsyy319TxLhdthQ6eKyCyHZj9Fj2R3MK+KByw+kMMyEl0nfQhxJ3w 0QCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713559349; x=1714164149; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YrZKOhtpV6MTNw07l3xNfXkFWnpKfDtuHjOgNMG4wsw=; b=sOp29EUE+Nalhbkgs8GyR0h5JAl+bQu2H/ZFqo6MRt7sLQ25wBbMuJ/i0BBgPGsS6V jicCWSrJw0i7b2M87F2glzVkIdHEpZR4RkEt7UzEQ93FnU/HCV4NU3mhoAZg5s7mjij8 ml2HOvCZdq+A8ibHEnnTZfSaLmovoBCczo2lCBPYoQ8QXYXGSXe/vouOvwJzdjgyO0Xv 4Oor2rNXuTJtivRU8DKQ3CLJrchyxZeDIUN4p5YtYPohpnWJI2flspOoTnB17fJd9wKl uBwmf7/UQHF0LtFMj0J9FWIcmx3DM9kSTXS+wA2TwKdExiesaBT2XTMuLp8ToQQBB9il s2pg== X-Forwarded-Encrypted: i=1; AJvYcCWjKBYlv+hKMX4RAKIC2bB/V4L4DW1PeJFEkRD1ED9KkpTHbS6zZERmAghn7PxPY10s2hyzQ7Zm8bTbkP/Y8SRGwlI= X-Gm-Message-State: AOJu0Yzl94V28nx0gPPrvBSxzREN3MgbmoNf50ubFKxcUicgAUJaz92L 3ctiEcsa5JE8+gLUmaIFuk/CkRVLoxMboRX+elH4rbBuG3chplpm5EQ/eNvTbNe/QeHwyBwsCEZ Y67Gisnh/bRSimdLC8bndrqwXQhOFKEx8K+tA X-Google-Smtp-Source: AGHT+IHGCKoZR7XY9wdfNo58Kxv4tWqevAIPExQUDfD5vPHJs9cnyNpUrdTiTvFLUAY+RU6mvZOuVNKZ1BbpFaUwkRA= X-Received: by 2002:a05:6e02:784:b0:36b:36d5:ca38 with SMTP id q4-20020a056e02078400b0036b36d5ca38mr4736ils.27.1713559348560; Fri, 19 Apr 2024 13:42:28 -0700 (PDT) MIME-Version: 1.0 References: <20240401232946.1837665-1-jthoughton@google.com> <20240401232946.1837665-4-jthoughton@google.com> In-Reply-To: From: James Houghton Date: Fri, 19 Apr 2024 13:41:52 -0700 Message-ID: Subject: Re: [PATCH v3 3/7] KVM: Add basic bitmap support into kvm_mmu_notifier_test/clear_young To: David Matlack Cc: Andrew Morton , Paolo Bonzini , Yu Zhao , Marc Zyngier , Oliver Upton , Sean Christopherson , Jonathan Corbet , James Morse , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , "H. Peter Anvin" , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Shaoqin Huang , Gavin Shan , Ricardo Koller , Raghavendra Rao Ananta , Ryan Roberts , David Rientjes , Axel Rasmussen , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, linux-mm@kvack.org, linux-trace-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: A9CF140004 X-Stat-Signature: 8p4mibyrk6gcf77mtndzsgc66o4xd139 X-Rspam-User: X-HE-Tag: 1713559349-763040 X-HE-Meta: U2FsdGVkX18MT4GCK3eH8Fqv6IiwKVNq3BiKAuZX88NQIKqlRpyyBC9UFjqVNNC/zs3skyqc0jJHCASg7ezQLmoTvry8644FQ3+d+QdtCyOHM0D0dyolF8k8F7aI+DVXRFTJqbgEleKV+w5ljeveDBuxKUIv3QetNkphnjSFPG6zsH/+KAfHcuCQdFSqfpJ5FA5cpd8LqRFPL+EiC+BcHD+qf4xsrCsNeLm2JPttixUkrvcLmHXdhatpdXaHD0aUaGncvrvaaGXRYhVezJIlnRB6mqpi0Y+RbM4fgTOgtJVcK451Ehl8oiUDhCSI+kGpO9QBnpoOn4zB7xbL3caIzYOS/1BrjKEAds55w4m8HD5ehHjgFyPalYFrtjly0UJ0+LalpOknGUeP0UHIPobnfBY/Lgs/Kti+O9NgzFweK1NJapymvlLMmhRMyUTNjEeJM5/Q90Ov3tgRoWIHtC4vNeSANVvi5peFvs8j4AC+9Az0z3QQ93DyYvm962WTtIopf9geO1SDcW6RsZUHuWr//wR/WN3x3U7TsbrLjy57jC1uXyeG2qtfqrq7sCGv+vBKruoE+9BZMwv23jS/3qz1li8uR0sw+66Ys6gVJf52Xw+ic3GkI2tdgjR6E19VzxUWwaWb5jZH5mdaCO3dQq3tu1jqU1UoRL2QaDg2R6jd3CcQ7aDrnpodArphgLe2uTic84O2GMi/p3XKJbPuFU6/ZhXdt7sS7dS0bfGAtk1QgZxeYKCuKMbFo/oetYh6OuTKVUsLxilGXSImF/xLK8AlaKnLR+sDgvycIDT2phokz8v63Jpm8QxiodUTranIefRTUzlXHqMiIeCjzZSlpS75ttUtLl6YnStQJL7MnNXleheGLcMowi1YweeoikT28rU4nVuP7ZXqtyJJeD0ohjR42vhhQOYeKmDg137prlMRB4+vTAuJqHhKODuIELAQshIWA8Y+2LWxmeWlmrnMZm8 wjtHhcLK uNfB6uusYfe6EIPnLBh+BC2qjc5Ig2flnuVbvQn/PP53V1fXh40CJPKRmhGNJB7A07Lo39QYHaQgtnQPlzDLG/i5c01/y1GpMnGofGdPrgXBWMsMHzIZ+Wpk8N3/ZjdzsKaNHwkJ8VR3CWyft6oL81By7K1tRzxq2d8qW1P2yT0+Q0U2ooqJ8AKJF8+m8/FWFY2Lqme5+hwiwuhAoEcJcUDBQrq0tE2HpIUwZzlkilJYNxdq/6V8seHxSZlsDP5/ahp5u06CribHZesnQHrPQecofjXmfdqIGBBDhEtgus5oSw9E= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Apr 12, 2024 at 1:28=E2=80=AFPM David Matlack = wrote: > > On 2024-04-01 11:29 PM, James Houghton wrote: > > Add kvm_arch_prepare_bitmap_age() for architectures to indiciate that > > they support bitmap-based aging in kvm_mmu_notifier_test_clear_young() > > and that they do not need KVM to grab the MMU lock for writing. This > > function allows architectures to do other locking or other preparatory > > work that it needs. > > There's a lot going on here. I know it's extra work but I think the > series would be easier to understand and simplify if you introduced the > KVM support for lockless test/clear_young() first, and then introduce > support for the bitmap-based look-around. Yeah I think this is the right thing to do. Thanks. > > Specifically: > > 1. Make all test/clear_young() notifiers lockless. i.e. Move the > mmu_lock into the architecture-specific code (kvm_age_gfn() and > kvm_test_age_gfn()). > > 2. Convert KVM/x86's kvm_{test,}_age_gfn() to be lockless for the TDP > MMU. > > 4. Convert KVM/arm64's kvm_{test,}_age_gfn() to hold the mmu_lock in > read-mode. > > 5. Add bitmap-based look-around support to KVM/x86 and KVM/arm64 > (probably 2-3 patches). This all sounds good to me. Thanks for laying it out for me -- this should be a lot simpler. > > > > > If an architecture does not implement kvm_arch_prepare_bitmap_age() or > > is unable to do bitmap-based aging at runtime (and marks the bitmap as > > unreliable): > > 1. If a bitmap was provided, we inform the caller that the bitmap is > > unreliable (MMU_NOTIFIER_YOUNG_BITMAP_UNRELIABLE). > > 2. If a bitmap was not provided, fall back to the old logic. > > > > Also add logic for architectures to easily use the provided bitmap if > > they are able. The expectation is that the architecture's implementatio= n > > of kvm_gfn_test_age() will use kvm_gfn_record_young(), and > > kvm_gfn_age() will use kvm_gfn_should_age(). > > > > Suggested-by: Yu Zhao > > Signed-off-by: James Houghton > > --- > > include/linux/kvm_host.h | 60 ++++++++++++++++++++++++++ > > virt/kvm/kvm_main.c | 92 +++++++++++++++++++++++++++++----------- > > 2 files changed, 127 insertions(+), 25 deletions(-) > > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > index 1800d03a06a9..5862fd7b5f9b 100644 > > --- a/include/linux/kvm_host.h > > +++ b/include/linux/kvm_host.h > > @@ -1992,6 +1992,26 @@ extern const struct _kvm_stats_desc kvm_vm_stats= _desc[]; > > extern const struct kvm_stats_header kvm_vcpu_stats_header; > > extern const struct _kvm_stats_desc kvm_vcpu_stats_desc[]; > > > > +/* > > + * Architectures that support using bitmaps for kvm_age_gfn() and > > + * kvm_test_age_gfn should return true for kvm_arch_prepare_bitmap_age= () > > + * and do any work they need to prepare. The subsequent walk will not > > + * automatically grab the KVM MMU lock, so some architectures may opt > > + * to grab it. > > + * > > + * If true is returned, a subsequent call to kvm_arch_finish_bitmap_ag= e() is > > + * guaranteed. > > + */ > > +#ifndef kvm_arch_prepare_bitmap_age > > +static inline bool kvm_arch_prepare_bitmap_age(struct mmu_notifier *mn= ) > > I find the name of these architecture callbacks misleading/confusing. > The lockless path is used even when a bitmap is not provided. i.e. > bitmap can be NULL in between kvm_arch_prepare/finish_bitmap_age(). Yes. I am really terrible at picking names.... I'm happy to just nix this, following your other suggestions. > > > +{ > > + return false; > > +} > > +#endif > > +#ifndef kvm_arch_finish_bitmap_age > > +static inline void kvm_arch_finish_bitmap_age(struct mmu_notifier *mn)= {} > > +#endif > > kvm_arch_finish_bitmap_age() seems unnecessary. I think the KVM/arm64 > code could acquire/release the mmu_lock in read-mode in > kvm_test_age_gfn() and kvm_age_gfn() right? Yes you're right, except that the way it is now, we only lock/unlock once for the notifier instead of once for each overlapping memslot, but that's not an issue, as you mention below. > > > + > > #ifdef CONFIG_KVM_GENERIC_MMU_NOTIFIER > > static inline struct kvm *mmu_notifier_to_kvm(struct mmu_notifier *mn) > > { > > @@ -2076,9 +2096,16 @@ static inline bool mmu_invalidate_retry_gfn_unsa= fe(struct kvm *kvm, > > return READ_ONCE(kvm->mmu_invalidate_seq) !=3D mmu_seq; > > } > > > > +struct test_clear_young_metadata { > > + unsigned long *bitmap; > > + unsigned long bitmap_offset_end; > > bitmap_offset_end is unused. Indeed, sorry about that. > > > + unsigned long end; > > + bool unreliable; > > +}; > > union kvm_mmu_notifier_arg { > > pte_t pte; > > unsigned long attributes; > > + struct test_clear_young_metadata *metadata; > > nit: Maybe s/metadata/test_clear_young/ ? Yes, that's better. > > > }; > > > > struct kvm_gfn_range { > > @@ -2087,11 +2114,44 @@ struct kvm_gfn_range { > > gfn_t end; > > union kvm_mmu_notifier_arg arg; > > bool may_block; > > + bool lockless; > > Please document this as it's somewhat subtle. A reader might think this > implies the entire operation runs without taking the mmu_lock. Will do, and I'll improve the comments for the other "lockless" variables. (In fact, it might be better to rename/adjust this one to "mmu_lock_taken" instead -- it's a little more obvious what that means.) > > > }; > > bool kvm_unmap_gfn_range(struct kvm *kvm, struct kvm_gfn_range *range)= ; > > bool kvm_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); > > bool kvm_test_age_gfn(struct kvm *kvm, struct kvm_gfn_range *range); > > bool kvm_set_spte_gfn(struct kvm *kvm, struct kvm_gfn_range *range); > > + > > +static inline void kvm_age_set_unreliable(struct kvm_gfn_range *range) > > +{ > > + struct test_clear_young_metadata *args =3D range->arg.metadata; > > + > > + args->unreliable =3D true; > > +} > > +static inline unsigned long kvm_young_bitmap_offset(struct kvm_gfn_ran= ge *range, > > + gfn_t gfn) > > +{ > > + struct test_clear_young_metadata *args =3D range->arg.metadata; > > + > > + return hva_to_gfn_memslot(args->end - 1, range->slot) - gfn; > > +} > > +static inline void kvm_gfn_record_young(struct kvm_gfn_range *range, g= fn_t gfn) > > +{ > > + struct test_clear_young_metadata *args =3D range->arg.metadata; > > + > > + WARN_ON_ONCE(gfn < range->start || gfn >=3D range->end); > > + if (args->bitmap) > > + __set_bit(kvm_young_bitmap_offset(range, gfn), args->bitm= ap); > > +} > > +static inline bool kvm_gfn_should_age(struct kvm_gfn_range *range, gfn= _t gfn) > > +{ > > + struct test_clear_young_metadata *args =3D range->arg.metadata; > > + > > + WARN_ON_ONCE(gfn < range->start || gfn >=3D range->end); > > + if (args->bitmap) > > + return test_bit(kvm_young_bitmap_offset(range, gfn), > > + args->bitmap); > > + return true; > > +} > > #endif > > > > #ifdef CONFIG_HAVE_KVM_IRQ_ROUTING > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index d0545d88c802..7d80321e2ece 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -550,6 +550,7 @@ struct kvm_mmu_notifier_range { > > on_lock_fn_t on_lock; > > bool flush_on_ret; > > bool may_block; > > + bool lockless; > > }; > > > > /* > > @@ -598,6 +599,8 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hv= a_range(struct kvm *kvm, > > struct kvm_memslots *slots; > > int i, idx; > > > > + BUILD_BUG_ON(sizeof(gfn_range.arg) !=3D sizeof(gfn_range.arg.pte)= ); > > + > > if (WARN_ON_ONCE(range->end <=3D range->start)) > > return r; > > > > @@ -637,15 +640,18 @@ static __always_inline kvm_mn_ret_t __kvm_handle_= hva_range(struct kvm *kvm, > > gfn_range.start =3D hva_to_gfn_memslot(hva_start,= slot); > > gfn_range.end =3D hva_to_gfn_memslot(hva_end + PA= GE_SIZE - 1, slot); > > gfn_range.slot =3D slot; > > + gfn_range.lockless =3D range->lockless; > > > > if (!r.found_memslot) { > > r.found_memslot =3D true; > > - KVM_MMU_LOCK(kvm); > > - if (!IS_KVM_NULL_FN(range->on_lock)) > > - range->on_lock(kvm); > > - > > - if (IS_KVM_NULL_FN(range->handler)) > > - break; > > + if (!range->lockless) { > > + KVM_MMU_LOCK(kvm); > > + if (!IS_KVM_NULL_FN(range->on_loc= k)) > > + range->on_lock(kvm); > > + > > + if (IS_KVM_NULL_FN(range->handler= )) > > + break; > > + } > > } > > r.ret |=3D range->handler(kvm, &gfn_range); > > } > > @@ -654,7 +660,7 @@ static __always_inline kvm_mn_ret_t __kvm_handle_hv= a_range(struct kvm *kvm, > > if (range->flush_on_ret && r.ret) > > kvm_flush_remote_tlbs(kvm); > > > > - if (r.found_memslot) > > + if (r.found_memslot && !range->lockless) > > KVM_MMU_UNLOCK(kvm); > > > > srcu_read_unlock(&kvm->srcu, idx); > > @@ -682,19 +688,24 @@ static __always_inline int kvm_handle_hva_range(s= truct mmu_notifier *mn, > > return __kvm_handle_hva_range(kvm, &range).ret; > > } > > > > -static __always_inline int kvm_handle_hva_range_no_flush(struct mmu_no= tifier *mn, > > - unsigned long st= art, > > - unsigned long en= d, > > - gfn_handler_t ha= ndler) > > +static __always_inline int kvm_handle_hva_range_no_flush( > > + struct mmu_notifier *mn, > > + unsigned long start, > > + unsigned long end, > > + gfn_handler_t handler, > > + union kvm_mmu_notifier_arg arg, > > + bool lockless) > > { > > struct kvm *kvm =3D mmu_notifier_to_kvm(mn); > > const struct kvm_mmu_notifier_range range =3D { > > .start =3D start, > > .end =3D end, > > .handler =3D handler, > > + .arg =3D arg, > > .on_lock =3D (void *)kvm_null_fn, > > .flush_on_ret =3D false, > > .may_block =3D false, > > + .lockless =3D lockless, > > }; > > > > return __kvm_handle_hva_range(kvm, &range).ret; > > @@ -909,15 +920,36 @@ static int kvm_mmu_notifier_clear_flush_young(str= uct mmu_notifier *mn, > > kvm_age_gfn); > > } > > > > -static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn, > > - struct mm_struct *mm, > > - unsigned long start, > > - unsigned long end, > > - unsigned long *bitmap) > > +static int kvm_mmu_notifier_test_clear_young(struct mmu_notifier *mn, > > + struct mm_struct *mm, > > + unsigned long start, > > + unsigned long end, > > + unsigned long *bitmap, > > + bool clear) > > Perhaps pass in the callback (kvm_test_age_gfn/kvm_age_gfn) instead of > true/false to avoid the naked booleans at the callsites? Will do. Thank you. > > > { > > - trace_kvm_age_hva(start, end); > > + if (kvm_arch_prepare_bitmap_age(mn)) { > > + struct test_clear_young_metadata args =3D { > > + .bitmap =3D bitmap, > > + .end =3D end, > > + .unreliable =3D false, > > + }; > > + union kvm_mmu_notifier_arg arg =3D { > > + .metadata =3D &args > > + }; > > + bool young; > > + > > + young =3D kvm_handle_hva_range_no_flush( > > + mn, start, end, > > + clear ? kvm_age_gfn : kvm_test_ag= e_gfn, > > + arg, true); > > I suspect the end result will be cleaner we make all architectures > lockless. i.e. Move the mmu_lock acquire/release into the > architecture-specific code. > > This could result in more acquire/release calls (one per memslot that > overlaps the provided range) but that should be a single memslot in the > majority of cases I think? > > Then unconditionally pass in the metadata structure. > > Then you don't need any special casing for the fast path / bitmap path. > The only thing needed is to figure out whether to return > MMU_NOTIFIER_YOUNG vs MMU_NOTIFIER_YOUNG_LOOK_AROUND and that can be > plumbed via test_clear_young_metadata or by changing gfn_handler_t to > return an int instead of a bool. Yes I think this simplification is a great idea. I agree that usually there will only be one memslot that overlaps a virtual address range in practice (MIN_LRU_BATCH is BITS_PER_LONG), so the theoretical additional locking/unlocking shouldn't be an issue. > > > + > > + kvm_arch_finish_bitmap_age(mn); > > > > - /* We don't support bitmaps. Don't test or clear anything. */ > > + if (!args.unreliable) > > + return young ? MMU_NOTIFIER_YOUNG_FAST : 0; > > + } > > + > > + /* A bitmap was passed but the architecture doesn't support bitma= ps */ > > if (bitmap) > > return MMU_NOTIFIER_YOUNG_BITMAP_UNRELIABLE; > > > > @@ -934,7 +966,21 @@ static int kvm_mmu_notifier_clear_young(struct mmu= _notifier *mn, > > * cadence. If we find this inaccurate, we might come up with a > > * more sophisticated heuristic later. > > */ > > - return kvm_handle_hva_range_no_flush(mn, start, end, kvm_age_gfn)= ; > > + return kvm_handle_hva_range_no_flush( > > + mn, start, end, clear ? kvm_age_gfn : kvm_test_ag= e_gfn, > > + KVM_MMU_NOTIFIER_NO_ARG, false); > > Should this return MMU_NOTIFIER_YOUNG explicitly? This code is assuming > MMU_NOTIFIER_YOUNG =3D=3D (int)true. Yes. Thank you for all the feedback!