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@xxxxxxxxx>
> ---
> 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
