* [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number
@ 2025-09-01 13:03 Ted Chen
2025-09-01 13:39 ` Ben Horgan
0 siblings, 1 reply; 4+ messages in thread
From: Ted Chen @ 2025-09-01 13:03 UTC (permalink / raw)
To: pbonzini; +Cc: kvm, Ted Chen
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());
mutex_lock(&kvm_debugfs_lock);
dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
if (dent) {
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number
2025-09-01 13:03 [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number Ted Chen
@ 2025-09-01 13:39 ` Ben Horgan
2025-09-02 12:46 ` Ted Chen
0 siblings, 1 reply; 4+ messages in thread
From: Ben Horgan @ 2025-09-01 13:39 UTC (permalink / raw)
To: Ted Chen, pbonzini; +Cc: kvm, Dave Martin
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number
2025-09-01 13:39 ` Ben Horgan
@ 2025-09-02 12:46 ` Ted Chen
2025-09-02 15:58 ` Dave Martin
0 siblings, 1 reply; 4+ messages in thread
From: Ted Chen @ 2025-09-02 12:46 UTC (permalink / raw)
To: Ben Horgan; +Cc: pbonzini, kvm, Dave Martin
On Mon, Sep 01, 2025 at 02:39:06PM +0100, Ben Horgan wrote:
> 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.
Good point. Maybe use timestamp instead?
So, we can know a bigger timestamp value corresponds to a VM created later.
Also since VMs are created in a single thread, they can't have the same
timestamp value.
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6c07dd423458..c3b0880be79a 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 * 4];
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-%llx", task_pid_nr(current),
+ fdname, ktime_get_ns());
mutex_lock(&kvm_debugfs_lock);
dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
if (dent) {
> Should the vm stats_id also be updated to be unique and use the same scheme?
I don't think so. Unlike debugfs paths that will be accessed directly by
userspace, the stats_id is used by anonymous inode files openned with unique
fd numbers in a single process. So, duplicated names should be ok.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number
2025-09-02 12:46 ` Ted Chen
@ 2025-09-02 15:58 ` Dave Martin
0 siblings, 0 replies; 4+ messages in thread
From: Dave Martin @ 2025-09-02 15:58 UTC (permalink / raw)
To: Ted Chen; +Cc: Ben Horgan, pbonzini, kvm
Hi,
On Tue, Sep 02, 2025 at 08:46:49PM +0800, Ted Chen wrote:
> On Mon, Sep 01, 2025 at 02:39:06PM +0100, Ben Horgan wrote:
> > 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.
> Good point. Maybe use timestamp instead?
> So, we can know a bigger timestamp value corresponds to a VM created later.
> Also since VMs are created in a single thread, they can't have the same
Why must all VMs be created in a single thread?
> timestamp value.
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6c07dd423458..c3b0880be79a 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 * 4];
Hmmm.
> 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-%llx", task_pid_nr(current),
> + fdname, ktime_get_ns());
> mutex_lock(&kvm_debugfs_lock);
> dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
> if (dent) {
>
>
> > Should the vm stats_id also be updated to be unique and use the same scheme?
>
> I don't think so. Unlike debugfs paths that will be accessed directly by
> userspace, the stats_id is used by anonymous inode files openned with unique
> fd numbers in a single process. So, duplicated names should be ok.
Just sticking my oar in here, since I'm interested in how we identify
VMs and vCPUs for use by other kernel subsystems:
The two proposed approaches may not guarantee uniqueness. The PID is
not unique because a single process can create multiple VMs. Possibly,
VMs can outlive the PID of the creating process (if a KVM fd is passed
to another process, and the original process terminates). The fd is
not unique because fds can be moved around with dup() and reallocated.
Random numbers are not unique (only _probably_ unique.)
If VMs are created from two threads simultaneously and if ownership of
kvm_debugfs_lock can move fast enough, or there is enough skid or
imprecision in the timestamp, then can ktime_get_ns() be non-unique too?
_Maybe_ this is good enough for debugfs -- which is often treated in a
best-effort way by kernel code.
However, if we really want a robust ID that can not only distinguish
between VMs but can also tell us reliably (and usefully) which is which
even when multiple VMs are created by a single process, I think KVM
userspace needs to be able to set and/or retrieve the ID, or part of it.
It might make sense to define a new kind of identifier for such
purposes.
I'm guessing that this is out of the scope for this patch, though.
Cheers
---Dave
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-02 15:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-01 13:03 [PATCH] KVM: Avoid debugfs warning caused by repeated vm fd number Ted Chen
2025-09-01 13:39 ` Ben Horgan
2025-09-02 12:46 ` Ted Chen
2025-09-02 15:58 ` Dave Martin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).