Re: [PATCH v5 6/6] KVM: guest_memfd: Drop superfluous WRITE_ONCE() when binding a memslot

[Sean Christopherson]

On Tue, Sep 22, 2026, Yan Zhao wrote:
> On Mon, Sep 21, 2026 at 05:13:32PM -0700, Sean Christopherson wrote:
> > Drop the superfluous WRITE_ONCE() when setting a memslot's guest_memfd file
> > during initial binding, as the memslot *must* be inactive and unreachable.
> > The superfluous WRITE_ONCE() was added by commit 67b43038ce14 ("KVM:
> > guest_memfd: Remove RCU-protected attribute from slot->gmem.file") to
> > maintain rough "parity" with the existing rcu_assign_pointer(), not
> > realizing that the only reason rcu_assign_pointer() was used was to make
> > sparse and other checkers happy.
> > 
> > Cc: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> > Reviewed-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> > Reviewed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >  virt/kvm/guest_memfd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 80932f4ec4a3..826d26036926 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -677,7 +677,7 @@ int kvm_gmem_prepare_memory_region(struct kvm *kvm, struct kvm_memory_slot *slot
> >  	 * kvm_gmem_bind() must occur on a new memslot.  Because the memslot
> >  	 * is not visible yet, kvm_gmem_get_pfn() is guaranteed to see the file.
> >  	 */
> > -	WRITE_ONCE(slot->gmem.file, file);
> > +	slot->gmem.file = file;
> >  	slot->gmem.pgoff = offset >> PAGE_SHIFT;
> >  	if (kvm_gmem_supports_mmap(inode))
> >  		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
> > 
> Thanks for the fix.
> Reviewed-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> 
> BTW: some questions regarding read/write to slot->gmem.file:
> 
> The WRITE_ONCE() in kvm_gmem_unbind() and kvm_gmem_release() are also invoked
> when the memslot is inactive and unreachable -- are they also superfluous?

The WRITE_ONCE() in release() is necessary, because the file could be freed/released
while it is still attached to a memslot.

I _think_ the one in unbind() is now superfluous after 0ee2c883b62d ("KVM:
guest_memfd: take the invalidate lock when unbinding a dying file"), but that one
needs more analysis.

> Do we need the READ_ONCE() in __kvm_gmem_get_pfn(), considering that other slot
> fields (e.g., slot->gmem.pgoff) are read without READ_ONCE()?

Yes, it's needed, because of the aforementioned release().  The other slot fields
are only ever modified when the slot is inactive, i.e. unreachable.  That's why
I think the unbind() WRITE_ONCE() is unnecessary; KVM should only unbind when the
slot is inactive.