Re: [PATCH v6 1/9] KVM: guest_memfd: Pass the number of pages instead of the end pfn into .invalidate()

[Ackerley Tng]

Sean Christopherson <seanjc@xxxxxxxxxx> writes:

> Pass the number of pages to "invalidate", i.e. reclaim, instead of the end
> pfn, as a first step towards aligning the function prototypes between the
> de facto "to private" and "to shared" arch hooks.  Eventually, the goal
> is to end up with kvm_gmem_arch_make_{private,shared}(), and in both cases,
> providing the number of pages makes the call sites slightly nicer, and also
> avoids any confusion over whether the end pfn is inclusive or exclusive.
>
> Opportunistically rename "start" to "pfn", again to align with the expected
> signature of make_private() (which needs to pass a starting gfn as well, at
> which point the "start" becomes noise).
>

I like ranges being defined as "start" and "nr_pages" because it removes
the ambiguity of whether "end" is inclusive or exclusive :)

>
> [...snip...]
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 171a9da1b685..ca1d93fb2495 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -530,11 +530,7 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
>  #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>  static void kvm_gmem_free_folio(struct folio *folio)
>  {
> -	struct page *page = folio_page(folio, 0);
> -	kvm_pfn_t pfn = page_to_pfn(page);
> -	int order = folio_order(folio);
> -
> -	kvm_arch_gmem_invalidate(pfn, pfn + (1ul << order));
> +	kvm_arch_gmem_invalidate(folio_file_pfn(folio, 0), folio_nr_pages(folio));

This could be just folio_pfn(folio) instead of having to pass the 0
index. The name of this function is free_folio() after all, so we'd be
invalidating (or later, reclaiming) the entire folio.

Reviewed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>

>  }
>  #endif
>
> --
> 2.55.0.229.g6434b31f56-goog