Add more context and information to the comment in kvm_gmem_release() that explains why there's no synchronization on RCU _or_ kvm->srcu. Point (b) from commit 67b43038ce14 ("KVM: guest_memfd: Remove RCU-protected attribute from slot->gmem.file") b) kvm->srcu ensures that kvm_gmem_unbind() and freeing of a memslot occur after the memslot is no longer visible to kvm_gmem_get_pfn(). is especially difficult to fully grok, particularly in light of commit ae431059e75d ("KVM: guest_memfd: Remove bindings on memslot deletion when gmem is dying"), which addressed a race between unbind() and release(). No functional change intended. Cc: Yan Zhao Cc: Vishal Annapurve Signed-off-by: Sean Christopherson --- virt/kvm/guest_memfd.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index fdaea3422c30..2e09d7ec0cfc 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -338,17 +338,25 @@ static int kvm_gmem_release(struct inode *inode, struct file *file) * dereferencing the slot for existing bindings needs to be protected * against memslot updates, specifically so that unbind doesn't race * and free the memslot (kvm_gmem_get_file() will return NULL). - * - * Since .release is called only when the reference count is zero, - * after which file_ref_get() and get_file_active() fail, - * kvm_gmem_get_pfn() cannot be using the file concurrently. - * file_ref_put() provides a full barrier, and get_file_active() the - * matching acquire barrier. */ mutex_lock(&kvm->slots_lock); filemap_invalidate_lock(inode->i_mapping); + /* + * Note! synchronize_srcu() is _not_ needed after nullifying memslot + * bindings as slot->gmem.file cannot be set back to a non-null value + * without the memslot first being deleted. I.e. this relies on the + * synchronize_srcu_expedited() in kvm_swap_active_memslots() to ensure + * kvm_gmem_get_pfn() (which runs with kvm->srcu held for read) can't + * grab a reference to slot->gmem.file even if the struct file object + * is reallocated. + * + * file_ref_put() provides a full barrier, and __get_file_rcu() the + * matching acquire barrier, to ensure that kvm_gmem_get_file() (via + * __get_file_rcu()) sees refcount==0 or fails the "file reloaded" + * check (file != NULL due to nullifying the file pointer here). + */ xa_for_each(&f->bindings, index, slot) WRITE_ONCE(slot->gmem.file, NULL); base-commit: 16ec4fb4ac95d878b879192d280db2baeec43272 -- 2.52.0.rc1.455.g30608eb744-goog