On Fri, Sep 09, 2022, Metin Kaya wrote: > It simplifies validation of gfn_to_hva_cache to make it less error prone Avoid pronouns as they're ambiguous, e.g. does "it" mean the helper or the patch? Obviously not a big deal in this case, but avoid pronouns is a good habit to get into. > per the discussion at > https://lore.kernel.org/all/4e29402770a7a254a1ea8ca8165af641ed0832ed.camel@xxxxxxxxxxxxx. Please write a proper changelog. Providing a link to the discussion is wonderful, but it's a supplement to a good changelog, not a substitution. > Signed-off-by: Metin Kaya <metikaya@xxxxxxxxxxxx> > --- > arch/x86/kvm/x86.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 43a6a7efc6ec..07d368dc69ad 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3425,11 +3425,22 @@ void kvm_service_local_tlb_flush_requests(struct kvm_vcpu *vcpu) > } > EXPORT_SYMBOL_GPL(kvm_service_local_tlb_flush_requests); > > +static inline bool kvm_gfn_to_hva_cache_valid(struct kvm *kvm, Don't add inline to local static functions, let the compiler make those decisions. > + struct gfn_to_hva_cache *ghc, > + gpa_t gpa) > +{ > + struct kvm_memslots *slots = kvm_memslots(kvm); > + > + return !unlikely(slots->generation != ghc->generation || I don't I don't think the unlikely should be here, it's the context in which the helper is used that determines whether or not the outcome is unlikely. > + gpa != ghc->gpa || > + kvm_is_error_hva(ghc->hva) || > + !ghc->memslot); Might be worth opportunistically reordering the checks to avoid grabbing memslots when something else is invalid, e.g. return gpa != ghc->gpa || kvm_is_error_hva(ghc->hva) || !ghc->memslot || kvm_memslots(kvm)->generation != ghc->generation); > +} > +
