On 12/12/24 00:36, Michael Roth wrote:
> Currently __kvm_gmem_get_pfn() sets 'is_prepared' so callers can skip
> calling kvm_gmem_prepare_folio(). However, subsequent patches will
> introduce some locking constraints around setting/checking preparedness
> that will require filemap_invalidate_lock*() to be held while checking
> for preparedness. This locking could theoretically be done inside
> __kvm_gmem_get_pfn(), or by requiring that filemap_invalidate_lock*() is
> held while calling __kvm_gmem_get_pfn(), but that places unnecessary
> constraints around when __kvm_gmem_get_pfn() can be called, whereas
> callers could just as easily call kvm_gmem_is_prepared() directly.
>
> So, in preparation for these locking changes, drop the 'is_prepared'
> argument, and leave it up to callers to handle checking preparedness
> where needed and with the proper locking constraints.
>
> Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
> ---
> virt/kvm/guest_memfd.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b69af3580bef..aa0038ddf4a4 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -773,7 +773,7 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
> static struct folio *__kvm_gmem_get_pfn(struct file *file,
> struct kvm_memory_slot *slot,
> pgoff_t index, kvm_pfn_t *pfn,
> - bool *is_prepared, int *max_order)
> + int *max_order)
> {
> struct kvm_gmem *gmem = file->private_data;
> struct folio *folio;
> @@ -803,7 +803,6 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
> if (max_order)
> *max_order = 0;
>
> - *is_prepared = kvm_gmem_is_prepared(file, index, folio);
> return folio;
> }
>
> @@ -814,19 +813,18 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> pgoff_t index = kvm_gmem_get_index(slot, gfn);
> struct file *file = kvm_gmem_get_file(slot);
> struct folio *folio;
> - bool is_prepared = false;
> int r = 0;
>
> if (!file)
> return -EFAULT;
>
> - folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
> + folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
> if (IS_ERR(folio)) {
> r = PTR_ERR(folio);
> goto out;
> }
>
> - if (!is_prepared)
> + if (kvm_gmem_is_prepared(file, index, folio))
Shouldn't this be !kvm_gmem_is_prepared() ?
Thanks,
Tom
> r = kvm_gmem_prepare_folio(kvm, file, slot, gfn, folio);
>
> folio_unlock(folio);
> @@ -872,7 +870,6 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> struct folio *folio;
> gfn_t gfn = start_gfn + i;
> pgoff_t index = kvm_gmem_get_index(slot, gfn);
> - bool is_prepared = false;
> kvm_pfn_t pfn;
>
> if (signal_pending(current)) {
> @@ -880,13 +877,13 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> break;
> }
>
> - folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, &max_order);
> + folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> if (IS_ERR(folio)) {
> ret = PTR_ERR(folio);
> break;
> }
>
> - if (is_prepared) {
> + if (kvm_gmem_is_prepared(file, index, folio)) {
> folio_unlock(folio);
> folio_put(folio);
> ret = -EEXIST;
