Re: [PATCH v3 2/5] KVM: Add file back-pointer to struct kvm

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Steffen Eiden <seiden@linux.ibm.com> writes:

> Add kvm->file as a no-reference back-pointer to the VM file in struct
> kvm. The pointer is set at VM creation time and cleared under
> WRITE_ONCE() in kvm_vm_release() before the file is freed.
> Callers storing the file must take their own reference.
>

I have a series that does something similar and also updates VFIO:

https://lore.kernel.org/all/20260917140159.1163281-2-aneesh.kumar@kernel.org

Any suggestions on how we can combine the two efforts? This series has
received more feedback from maintainers.

>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Steffen Eiden <seiden@linux.ibm.com>
> ---
>  include/linux/kvm_host.h | 6 ++++++
>  virt/kvm/kvm_main.c      | 4 ++++
>  2 files changed, 10 insertions(+)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index e840ceda9f16..943b108f01cd 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -784,6 +784,12 @@ struct kvm {
>  	 * kvm_swap_active_memslots().
>  	 */
>  	struct mutex slots_arch_lock;
> +	/*
> +	 * Back-reference to the VM file for subsystems (e.g., VFIO). Holds no
> +	 * reference to avoid pinning the VM. Callers storing the file must
> +	 * take their own reference.
> +	 */
> +	struct file *file;
>

I had to add the file pointer as an __rcu field:

@@ -840,6 +842,7 @@ struct kvm {
	struct srcu_struct srcu;
	struct srcu_struct irq_srcu;
	pid_t userspace_pid;
+	struct file __rcu *_file;
	bool override_halt_poll_ns;
	unsigned int max_halt_poll_ns;
	u32 dirty_ring_size;

because my series uses get_file_active().

>  	struct mm_struct *mm; /* userspace tied to this vm */
>  	unsigned long nr_memslot_pages;
>  	/* The two memslot sets - active and inactive (per address space) */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0172ca94f82e..bab71d398236 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1352,6 +1352,8 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
>  
>  	kvm_irqfd_release(kvm);
>  
> +	WRITE_ONCE(kvm->file, NULL);
> +
>  	kvm_put_kvm(kvm);
>  	return 0;
>  }
> @@ -5532,6 +5534,8 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
>  		goto put_kvm;
>  	}
>  
> +	kvm->file = file;
> +
>  	/*
>  	 * Don't call kvm_put_kvm anymore at this point; file->f_op is
>  	 * already set, with ->release() being kvm_vm_release().  In error
>

-aneesh