Re: [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number

[Ben Horgan <ben.horgan@arm.com>]

Hi Ted,

On 9/1/25 14:03, Ted Chen wrote:
> Avoid debugfs warning like "KVM: debugfs: duplicate directory 59904-4"
> caused by creating VMs with the same vm fd number in a single process.
> 
> As shown in the below test case, two test() are executed sequentially in a
> single process, each creating a new VM.
> 
> Though the 2nd test() creates a new VM after the 1st test() closes the
> vm_fd, KVM prints warnings like "KVM: debugfs: duplicate directory 59904-4"
> on creating the 2nd VM.
> 
> This is due to the dup() of the vcpu_fd in test(). So, after closing the
> 1st vm_fd, kvm->users_count of the 1st VM is still > 0 when creating the
> 2nd VM. So, KVM has not yet invoked kvm_destroy_vm() and
> kvm_destroy_vm_debugfs() for the 1st VM after closing the 1st vm_fd. The
> 2nd test() thus will be able to create a different VM with the same vm fd
> number as the 1st VM.
> 
> Therefore, besides having "pid" and "fdname" in the dir_name of the
> debugfs, add a random number to differentiate different VMs to avoid
> printing warning, also allowing the 2nd VM to have a functional debugfs.
> 
> Use get_random_u32() to avoid dir_name() taking up too much memory while
> greatly reducing the chance of printing warning.
> 
> void test(void)
> {
>         int kvm_fd, vm_fd, vcpu_fd;
> 
>         kvm_fd = open("/dev/kvm", O_RDWR);
>         if (kvm_fd == -1)
>                 return;
> 
>         vm_fd = ioctl(kvm_fd, KVM_CREATE_VM, 0);
>         if (vm_fd == -1)
>                 return;
>         vcpu_fd = ioctl(vm_fd, KVM_CREATE_VCPU, 0);
>         if (vcpu_fd == -1)
>                 return;
> 
>         dup(vcpu_fd);
>         close(vcpu_fd);
>         close(vm_fd);
>         close(kvm_fd);
> }
> 
> int main()
> {
>         test();
>         test();
> 
>         return 0;
> }
> 
> Signed-off-by: Ted Chen <znscnchen@gmail.com>
> ---
>  virt/kvm/kvm_main.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6c07dd423458..f92a60ed5de8 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1017,7 +1017,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, const char *fdname)
>  {
>  	static DEFINE_MUTEX(kvm_debugfs_lock);
>  	struct dentry *dent;
> -	char dir_name[ITOA_MAX_LEN * 2];
> +	char dir_name[ITOA_MAX_LEN * 3];
>  	struct kvm_stat_data *stat_data;
>  	const struct _kvm_stats_desc *pdesc;
>  	int i, ret = -ENOMEM;
> @@ -1027,7 +1027,8 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, const char *fdname)
>  	if (!debugfs_initialized())
>  		return 0;
>  
> -	snprintf(dir_name, sizeof(dir_name), "%d-%s", task_pid_nr(current), fdname);
> +	snprintf(dir_name, sizeof(dir_name), "%d-%s-%u", task_pid_nr(current),
> +		 fdname, get_random_u32());

This does make the directory names (very likely) to be unique but it's
not helpful in distinguishing which directory maps to which vm. I wonder
if there is some better id we could use here.

Should the vm stats_id also be updated to be unique and use the same scheme?

>  	mutex_lock(&kvm_debugfs_lock);
>  	dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
>  	if (dent) {

Thanks,

Ben