Invert the order of KVM's page/pfn release helpers so that the "inner" helper operates on a page instead of a pfn. As pointed out by Linus[*], converting between struct page and a pfn isn't necessarily cheap, and that's not even counting the overhead of is_error_noslot_pfn() and kvm_is_reserved_pfn(). Even if the checks were dirt cheap, there's no reason to convert from a page to a pfn and back to a page, just to mark the page dirty/accessed or to put a reference to the page. Opportunistically drop a stale declaration of kvm_set_page_accessed() from kvm_host.h (there was no implementation). No functional change intended. [*] https://lore.kernel.org/all/CAHk-=wifQimj2d6npq-wCi5onYPjzQg4vyO4tFcPJJZr268cRw@xxxxxxxxxxxxxx Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> --- include/linux/kvm_host.h | 1 - virt/kvm/kvm_main.c | 58 +++++++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 252ee4a61b58..e32fbde79298 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1116,7 +1116,6 @@ unsigned long gfn_to_hva_memslot_prot(struct kvm_memory_slot *slot, gfn_t gfn, bool *writable); void kvm_release_page_clean(struct page *page); void kvm_release_page_dirty(struct page *page); -void kvm_set_page_accessed(struct page *page); kvm_pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn); kvm_pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault, diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 46d12998732e..ab7549195c68 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2798,18 +2798,40 @@ struct page *kvm_vcpu_gfn_to_page(struct kvm_vcpu *vcpu, gfn_t gfn) } EXPORT_SYMBOL_GPL(kvm_vcpu_gfn_to_page); +static bool kvm_is_ad_tracked_page(struct page *page) +{ + /* + * Per page-flags.h, pages tagged PG_reserved "should in general not be + * touched (e.g. set dirty) except by its owner". + */ + return !PageReserved(page); +} + +static void kvm_set_page_dirty(struct page *page) +{ + if (kvm_is_ad_tracked_page(page)) + SetPageDirty(page); +} + +static void kvm_set_page_accessed(struct page *page) +{ + if (kvm_is_ad_tracked_page(page)) + mark_page_accessed(page); +} + void kvm_release_page_clean(struct page *page) { WARN_ON(is_error_page(page)); - kvm_release_pfn_clean(page_to_pfn(page)); + kvm_set_page_accessed(page); + put_page(page); } EXPORT_SYMBOL_GPL(kvm_release_page_clean); void kvm_release_pfn_clean(kvm_pfn_t pfn) { if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn)) - put_page(pfn_to_page(pfn)); + kvm_release_page_clean(pfn_to_page(pfn)); } EXPORT_SYMBOL_GPL(kvm_release_pfn_clean); @@ -2817,40 +2839,34 @@ void kvm_release_page_dirty(struct page *page) { WARN_ON(is_error_page(page)); - kvm_release_pfn_dirty(page_to_pfn(page)); + kvm_set_page_dirty(page); + kvm_release_page_clean(page); } EXPORT_SYMBOL_GPL(kvm_release_page_dirty); void kvm_release_pfn_dirty(kvm_pfn_t pfn) { - kvm_set_pfn_dirty(pfn); - kvm_release_pfn_clean(pfn); + if (!is_error_noslot_pfn(pfn) && !kvm_is_reserved_pfn(pfn)) + kvm_release_page_dirty(pfn_to_page(pfn)); } EXPORT_SYMBOL_GPL(kvm_release_pfn_dirty); -static bool kvm_is_ad_tracked_pfn(kvm_pfn_t pfn) -{ - if (!pfn_valid(pfn)) - return false; - - /* - * Per page-flags.h, pages tagged PG_reserved "should in general not be - * touched (e.g. set dirty) except by its owner". - */ - return !PageReserved(pfn_to_page(pfn)); -} - +/* + * Note, checking for an error/noslot pfn is the caller's responsibility when + * directly marking a page dirty/accessed. Unlike the "release" helpers, the + * "set" helpers are not to be unused when the pfn might point at garbage. + */ void kvm_set_pfn_dirty(kvm_pfn_t pfn) { - if (kvm_is_ad_tracked_pfn(pfn)) - SetPageDirty(pfn_to_page(pfn)); + if (pfn_valid(pfn)) + kvm_set_page_dirty(pfn_to_page(pfn)); } EXPORT_SYMBOL_GPL(kvm_set_pfn_dirty); void kvm_set_pfn_accessed(kvm_pfn_t pfn) { - if (kvm_is_ad_tracked_pfn(pfn)) - mark_page_accessed(pfn_to_page(pfn)); + if (pfn_valid(pfn)) + kvm_set_page_accessed(pfn_to_page(pfn)); } EXPORT_SYMBOL_GPL(kvm_set_pfn_accessed); -- 2.36.0.464.gb9c8b46e94-goog
