* [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
@ 2016-10-14 6:02 Suraj Jitindar Singh
2016-10-17 13:40 ` Radim Krčmář
2016-10-17 16:50 ` Paolo Bonzini
0 siblings, 2 replies; 7+ messages in thread
From: Suraj Jitindar Singh @ 2016-10-14 6:02 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, Suraj Jitindar Singh
Various kvm vm and vcpu stats are provided via debugfs entries.
Currently there is no way to reset these stats back to zero for the system
except by stopping all vms.
Add the ability to clear (reset back to zero) these stats on a per stat
basis by writing to the debugfs files. The stats are just reset to zero
irrespective of what is actually written to the file.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
virt/kvm/kvm_main.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 60 insertions(+), 6 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 81dfc73..141fc03 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -595,7 +595,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
stat_data->kvm = kvm;
stat_data->offset = p->offset;
kvm->debugfs_stat_data[p - debugfs_entries] = stat_data;
- if (!debugfs_create_file(p->name, 0444,
+ if (!debugfs_create_file(p->name, 0644,
kvm->debugfs_dentry,
stat_data,
stat_fops_per_vm[p->kind]))
@@ -3658,11 +3658,20 @@ static int vm_stat_get_per_vm(void *data, u64 *val)
return 0;
}
+static int vm_stat_clear_per_vm(void *data, u64 val)
+{
+ struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data;
+
+ *(ulong *)((void *)stat_data->kvm + stat_data->offset) = 0;
+
+ return 0;
+}
+
static int vm_stat_get_per_vm_open(struct inode *inode, struct file *file)
{
__simple_attr_check_format("%llu\n", 0ull);
return kvm_debugfs_open(inode, file, vm_stat_get_per_vm,
- NULL, "%llu\n");
+ vm_stat_clear_per_vm, "%llu\n");
}
static const struct file_operations vm_stat_get_per_vm_fops = {
@@ -3688,11 +3697,23 @@ static int vcpu_stat_get_per_vm(void *data, u64 *val)
return 0;
}
+static int vcpu_stat_clear_per_vm(void *data, u64 val)
+{
+ int i;
+ struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data;
+ struct kvm_vcpu *vcpu;
+
+ kvm_for_each_vcpu(i, vcpu, stat_data->kvm)
+ *(u64 *)((void *)vcpu + stat_data->offset) = 0;
+
+ return 0;
+}
+
static int vcpu_stat_get_per_vm_open(struct inode *inode, struct file *file)
{
__simple_attr_check_format("%llu\n", 0ull);
return kvm_debugfs_open(inode, file, vcpu_stat_get_per_vm,
- NULL, "%llu\n");
+ vcpu_stat_clear_per_vm, "%llu\n");
}
static const struct file_operations vcpu_stat_get_per_vm_fops = {
@@ -3727,7 +3748,23 @@ static int vm_stat_get(void *_offset, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, NULL, "%llu\n");
+static int vm_stat_clear(void *_offset, u64 val)
+{
+ unsigned offset = (long)_offset;
+ struct kvm *kvm;
+ struct kvm_stat_data stat_tmp = {.offset = offset};
+
+ spin_lock(&kvm_lock);
+ list_for_each_entry(kvm, &vm_list, vm_list) {
+ stat_tmp.kvm = kvm;
+ vm_stat_clear_per_vm((void *)&stat_tmp, 0);
+ }
+ spin_unlock(&kvm_lock);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, vm_stat_clear, "%llu\n");
static int vcpu_stat_get(void *_offset, u64 *val)
{
@@ -3747,7 +3784,24 @@ static int vcpu_stat_get(void *_offset, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get, NULL, "%llu\n");
+static int vcpu_stat_clear(void *_offset, u64 val)
+{
+ unsigned offset = (long)_offset;
+ struct kvm *kvm;
+ struct kvm_stat_data stat_tmp = {.offset = offset};
+
+ spin_lock(&kvm_lock);
+ list_for_each_entry(kvm, &vm_list, vm_list) {
+ stat_tmp.kvm = kvm;
+ vcpu_stat_clear_per_vm((void *)&stat_tmp, 0);
+ }
+ spin_unlock(&kvm_lock);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get, vcpu_stat_clear,
+ "%llu\n");
static const struct file_operations *stat_fops[] = {
[KVM_STAT_VCPU] = &vcpu_stat_fops,
@@ -3765,7 +3819,7 @@ static int kvm_init_debug(void)
kvm_debugfs_num_entries = 0;
for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) {
- if (!debugfs_create_file(p->name, 0444, kvm_debugfs_dir,
+ if (!debugfs_create_file(p->name, 0644, kvm_debugfs_dir,
(void *)(long)p->offset,
stat_fops[p->kind]))
goto out_dir;
--
2.5.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-14 6:02 [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry Suraj Jitindar Singh
@ 2016-10-17 13:40 ` Radim Krčmář
2016-10-17 13:48 ` Christian Borntraeger
2016-10-18 7:16 ` Suraj Jitindar Singh
2016-10-17 16:50 ` Paolo Bonzini
1 sibling, 2 replies; 7+ messages in thread
From: Radim Krčmář @ 2016-10-17 13:40 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, pbonzini
2016-10-14 17:02+1100, Suraj Jitindar Singh:
> Various kvm vm and vcpu stats are provided via debugfs entries.
> Currently there is no way to reset these stats back to zero for the system
> except by stopping all vms.
It is not really resetting, just a special case of current behavior --
top-level stats are a sum of all existing VMs/VCPUs, so we would see 0
if they all had 0.
> Add the ability to clear (reset back to zero) these stats on a per stat
> basis by writing to the debugfs files.
I can see this being used before a test, but the userspace could also
read the value instead of writing and subtract it from the value after
the test to get a similar result.
The userspace-only solution is insufficient?
When and what do you zero?
> The stats are just reset to zero
> irrespective of what is actually written to the file.
Sounds good. Setting the val would be as simple, but there shouldn't be
a reason to and it is a debug interface, so we can change it anytime.
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
I have applied it to kvm/queue for the moment, thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-17 13:40 ` Radim Krčmář
@ 2016-10-17 13:48 ` Christian Borntraeger
2016-10-18 7:18 ` Suraj Jitindar Singh
2016-10-18 7:16 ` Suraj Jitindar Singh
1 sibling, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2016-10-17 13:48 UTC (permalink / raw)
To: Radim Krčmář, Suraj Jitindar Singh; +Cc: kvm, pbonzini
On 10/17/2016 03:40 PM, Radim Krčmář wrote:
> 2016-10-14 17:02+1100, Suraj Jitindar Singh:
>> Various kvm vm and vcpu stats are provided via debugfs entries.
>> Currently there is no way to reset these stats back to zero for the system
>> except by stopping all vms.
>
> It is not really resetting, just a special case of current behavior --
> top-level stats are a sum of all existing VMs/VCPUs, so we would see 0
> if they all had 0.
>
>> Add the ability to clear (reset back to zero) these stats on a per stat
>> basis by writing to the debugfs files.
>
> I can see this being used before a test, but the userspace could also
> read the value instead of writing and subtract it from the value after
> the test to get a similar result.
>
> The userspace-only solution is insufficient?
>
> When and what do you zero?
>
>> The stats are just reset to zero
>> irrespective of what is actually written to the file.
>
> Sounds good. Setting the val would be as simple, but there shouldn't be
> a reason to and it is a debug interface, so we can change it anytime.
>
>> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>> ---
>
> I have applied it to kvm/queue for the moment, thanks.
FWIW, I was thinking about another change: provide the accumulated values of
the counters of all VMs that have been destroyed (basically on vm_destroy read
the values and add them on top of the "forever" counters.
Did not find the time to do something like this.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-14 6:02 [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry Suraj Jitindar Singh
2016-10-17 13:40 ` Radim Krčmář
@ 2016-10-17 16:50 ` Paolo Bonzini
2016-10-18 6:20 ` Suraj Jitindar Singh
1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2016-10-17 16:50 UTC (permalink / raw)
To: Suraj Jitindar Singh; +Cc: kvm, rkrcmar
----- Original Message -----
> From: "Suraj Jitindar Singh" <sjitindarsingh@gmail.com>
> To: kvm@vger.kernel.org
> Cc: pbonzini@redhat.com, rkrcmar@redhat.com, "Suraj Jitindar Singh" <sjitindarsingh@gmail.com>
> Sent: Friday, October 14, 2016 8:02:42 AM
> Subject: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
>
> Various kvm vm and vcpu stats are provided via debugfs entries.
> Currently there is no way to reset these stats back to zero for the system
> except by stopping all vms.
>
> Add the ability to clear (reset back to zero) these stats on a per stat
> basis by writing to the debugfs files. The stats are just reset to zero
> irrespective of what is actually written to the file.
Even though this is a debug interface, people will still rely on
underspecified APIs. Please make it fail with EINVAL unless 0 is
written. Apart from this, it's a good idea.
Paolo
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
> virt/kvm/kvm_main.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 60 insertions(+), 6 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 81dfc73..141fc03 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -595,7 +595,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
> stat_data->kvm = kvm;
> stat_data->offset = p->offset;
> kvm->debugfs_stat_data[p - debugfs_entries] = stat_data;
> - if (!debugfs_create_file(p->name, 0444,
> + if (!debugfs_create_file(p->name, 0644,
> kvm->debugfs_dentry,
> stat_data,
> stat_fops_per_vm[p->kind]))
> @@ -3658,11 +3658,20 @@ static int vm_stat_get_per_vm(void *data, u64 *val)
> return 0;
> }
>
> +static int vm_stat_clear_per_vm(void *data, u64 val)
> +{
> + struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data;
> +
> + *(ulong *)((void *)stat_data->kvm + stat_data->offset) = 0;
> +
> + return 0;
> +}
> +
> static int vm_stat_get_per_vm_open(struct inode *inode, struct file *file)
> {
> __simple_attr_check_format("%llu\n", 0ull);
> return kvm_debugfs_open(inode, file, vm_stat_get_per_vm,
> - NULL, "%llu\n");
> + vm_stat_clear_per_vm, "%llu\n");
> }
>
> static const struct file_operations vm_stat_get_per_vm_fops = {
> @@ -3688,11 +3697,23 @@ static int vcpu_stat_get_per_vm(void *data, u64 *val)
> return 0;
> }
>
> +static int vcpu_stat_clear_per_vm(void *data, u64 val)
> +{
> + int i;
> + struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data;
> + struct kvm_vcpu *vcpu;
> +
> + kvm_for_each_vcpu(i, vcpu, stat_data->kvm)
> + *(u64 *)((void *)vcpu + stat_data->offset) = 0;
> +
> + return 0;
> +}
> +
> static int vcpu_stat_get_per_vm_open(struct inode *inode, struct file *file)
> {
> __simple_attr_check_format("%llu\n", 0ull);
> return kvm_debugfs_open(inode, file, vcpu_stat_get_per_vm,
> - NULL, "%llu\n");
> + vcpu_stat_clear_per_vm, "%llu\n");
> }
>
> static const struct file_operations vcpu_stat_get_per_vm_fops = {
> @@ -3727,7 +3748,23 @@ static int vm_stat_get(void *_offset, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, NULL, "%llu\n");
> +static int vm_stat_clear(void *_offset, u64 val)
> +{
> + unsigned offset = (long)_offset;
> + struct kvm *kvm;
> + struct kvm_stat_data stat_tmp = {.offset = offset};
> +
> + spin_lock(&kvm_lock);
> + list_for_each_entry(kvm, &vm_list, vm_list) {
> + stat_tmp.kvm = kvm;
> + vm_stat_clear_per_vm((void *)&stat_tmp, 0);
> + }
> + spin_unlock(&kvm_lock);
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, vm_stat_clear, "%llu\n");
>
> static int vcpu_stat_get(void *_offset, u64 *val)
> {
> @@ -3747,7 +3784,24 @@ static int vcpu_stat_get(void *_offset, u64 *val)
> return 0;
> }
>
> -DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get, NULL, "%llu\n");
> +static int vcpu_stat_clear(void *_offset, u64 val)
> +{
> + unsigned offset = (long)_offset;
> + struct kvm *kvm;
> + struct kvm_stat_data stat_tmp = {.offset = offset};
> +
> + spin_lock(&kvm_lock);
> + list_for_each_entry(kvm, &vm_list, vm_list) {
> + stat_tmp.kvm = kvm;
> + vcpu_stat_clear_per_vm((void *)&stat_tmp, 0);
> + }
> + spin_unlock(&kvm_lock);
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get, vcpu_stat_clear,
> + "%llu\n");
>
> static const struct file_operations *stat_fops[] = {
> [KVM_STAT_VCPU] = &vcpu_stat_fops,
> @@ -3765,7 +3819,7 @@ static int kvm_init_debug(void)
>
> kvm_debugfs_num_entries = 0;
> for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) {
> - if (!debugfs_create_file(p->name, 0444, kvm_debugfs_dir,
> + if (!debugfs_create_file(p->name, 0644, kvm_debugfs_dir,
> (void *)(long)p->offset,
> stat_fops[p->kind]))
> goto out_dir;
> --
> 2.5.5
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-17 16:50 ` Paolo Bonzini
@ 2016-10-18 6:20 ` Suraj Jitindar Singh
0 siblings, 0 replies; 7+ messages in thread
From: Suraj Jitindar Singh @ 2016-10-18 6:20 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, rkrcmar
On Mon, 2016-10-17 at 12:50 -0400, Paolo Bonzini wrote:
>
> ----- Original Message -----
> >
> > From: "Suraj Jitindar Singh" <sjitindarsingh@gmail.com>
> > To: kvm@vger.kernel.org
> > Cc: pbonzini@redhat.com, rkrcmar@redhat.com, "Suraj Jitindar Singh"
> > <sjitindarsingh@gmail.com>
> > Sent: Friday, October 14, 2016 8:02:42 AM
> > Subject: [PATCH] kvm/stats: Update kvm stats to clear on write to
> > their debugfs entry
> >
> > Various kvm vm and vcpu stats are provided via debugfs entries.
> > Currently there is no way to reset these stats back to zero for the
> > system
> > except by stopping all vms.
> >
> > Add the ability to clear (reset back to zero) these stats on a per
> > stat
> > basis by writing to the debugfs files. The stats are just reset to
> > zero
> > irrespective of what is actually written to the file.
> Even though this is a debug interface, people will still rely on
> underspecified APIs. Please make it fail with EINVAL unless 0 is
> written. Apart from this, it's a good idea.
Ok, I'll update this and send a V2
>
> Paolo
>
> >
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> > virt/kvm/kvm_main.c | 66
> > ++++++++++++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 60 insertions(+), 6 deletions(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 81dfc73..141fc03 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -595,7 +595,7 @@ static int kvm_create_vm_debugfs(struct kvm
> > *kvm, int fd)
> > stat_data->kvm = kvm;
> > stat_data->offset = p->offset;
> > kvm->debugfs_stat_data[p - debugfs_entries] =
> > stat_data;
> > - if (!debugfs_create_file(p->name, 0444,
> > + if (!debugfs_create_file(p->name, 0644,
> > kvm->debugfs_dentry,
> > stat_data,
> > stat_fops_per_vm[p-
> > >kind]))
> > @@ -3658,11 +3658,20 @@ static int vm_stat_get_per_vm(void *data,
> > u64 *val)
> > return 0;
> > }
> >
> > +static int vm_stat_clear_per_vm(void *data, u64 val)
> > +{
> > + struct kvm_stat_data *stat_data = (struct kvm_stat_data
> > *)data;
> > +
> > + *(ulong *)((void *)stat_data->kvm + stat_data->offset) =
> > 0;
> > +
> > + return 0;
> > +}
> > +
> > static int vm_stat_get_per_vm_open(struct inode *inode, struct
> > file *file)
> > {
> > __simple_attr_check_format("%llu\n", 0ull);
> > return kvm_debugfs_open(inode, file, vm_stat_get_per_vm,
> > - NULL, "%llu\n");
> > + vm_stat_clear_per_vm, "%llu\n");
> > }
> >
> > static const struct file_operations vm_stat_get_per_vm_fops = {
> > @@ -3688,11 +3697,23 @@ static int vcpu_stat_get_per_vm(void *data,
> > u64 *val)
> > return 0;
> > }
> >
> > +static int vcpu_stat_clear_per_vm(void *data, u64 val)
> > +{
> > + int i;
> > + struct kvm_stat_data *stat_data = (struct kvm_stat_data
> > *)data;
> > + struct kvm_vcpu *vcpu;
> > +
> > + kvm_for_each_vcpu(i, vcpu, stat_data->kvm)
> > + *(u64 *)((void *)vcpu + stat_data->offset) = 0;
> > +
> > + return 0;
> > +}
> > +
> > static int vcpu_stat_get_per_vm_open(struct inode *inode, struct
> > file *file)
> > {
> > __simple_attr_check_format("%llu\n", 0ull);
> > return kvm_debugfs_open(inode, file, vcpu_stat_get_per_vm,
> > - NULL, "%llu\n");
> > + vcpu_stat_clear_per_vm,
> > "%llu\n");
> > }
> >
> > static const struct file_operations vcpu_stat_get_per_vm_fops = {
> > @@ -3727,7 +3748,23 @@ static int vm_stat_get(void *_offset, u64
> > *val)
> > return 0;
> > }
> >
> > -DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, NULL,
> > "%llu\n");
> > +static int vm_stat_clear(void *_offset, u64 val)
> > +{
> > + unsigned offset = (long)_offset;
> > + struct kvm *kvm;
> > + struct kvm_stat_data stat_tmp = {.offset = offset};
> > +
> > + spin_lock(&kvm_lock);
> > + list_for_each_entry(kvm, &vm_list, vm_list) {
> > + stat_tmp.kvm = kvm;
> > + vm_stat_clear_per_vm((void *)&stat_tmp, 0);
> > + }
> > + spin_unlock(&kvm_lock);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SIMPLE_ATTRIBUTE(vm_stat_fops, vm_stat_get, vm_stat_clear,
> > "%llu\n");
> >
> > static int vcpu_stat_get(void *_offset, u64 *val)
> > {
> > @@ -3747,7 +3784,24 @@ static int vcpu_stat_get(void *_offset, u64
> > *val)
> > return 0;
> > }
> >
> > -DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get, NULL,
> > "%llu\n");
> > +static int vcpu_stat_clear(void *_offset, u64 val)
> > +{
> > + unsigned offset = (long)_offset;
> > + struct kvm *kvm;
> > + struct kvm_stat_data stat_tmp = {.offset = offset};
> > +
> > + spin_lock(&kvm_lock);
> > + list_for_each_entry(kvm, &vm_list, vm_list) {
> > + stat_tmp.kvm = kvm;
> > + vcpu_stat_clear_per_vm((void *)&stat_tmp, 0);
> > + }
> > + spin_unlock(&kvm_lock);
> > +
> > + return 0;
> > +}
> > +
> > +DEFINE_SIMPLE_ATTRIBUTE(vcpu_stat_fops, vcpu_stat_get,
> > vcpu_stat_clear,
> > + "%llu\n");
> >
> > static const struct file_operations *stat_fops[] = {
> > [KVM_STAT_VCPU] = &vcpu_stat_fops,
> > @@ -3765,7 +3819,7 @@ static int kvm_init_debug(void)
> >
> > kvm_debugfs_num_entries = 0;
> > for (p = debugfs_entries; p->name; ++p,
> > kvm_debugfs_num_entries++) {
> > - if (!debugfs_create_file(p->name, 0444,
> > kvm_debugfs_dir,
> > + if (!debugfs_create_file(p->name, 0644,
> > kvm_debugfs_dir,
> > (void *)(long)p->offset,
> > stat_fops[p->kind]))
> > goto out_dir;
> > --
> > 2.5.5
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-17 13:40 ` Radim Krčmář
2016-10-17 13:48 ` Christian Borntraeger
@ 2016-10-18 7:16 ` Suraj Jitindar Singh
1 sibling, 0 replies; 7+ messages in thread
From: Suraj Jitindar Singh @ 2016-10-18 7:16 UTC (permalink / raw)
To: Radim Krčmář; +Cc: kvm, pbonzini
On Mon, 2016-10-17 at 15:40 +0200, Radim Krčmář wrote:
> 2016-10-14 17:02+1100, Suraj Jitindar Singh:
> >
> > Various kvm vm and vcpu stats are provided via debugfs entries.
> > Currently there is no way to reset these stats back to zero for the
> > system
> > except by stopping all vms.
> It is not really resetting, just a special case of current behavior
> --
> top-level stats are a sum of all existing VMs/VCPUs, so we would see
> 0
> if they all had 0.
Yeah I worded that poorly
>
> >
> > Add the ability to clear (reset back to zero) these stats on a per
> > stat
> > basis by writing to the debugfs files.
> I can see this being used before a test, but the userspace could also
> read the value instead of writing and subtract it from the value
> after
> the test to get a similar result.
True
>
> The userspace-only solution is insufficient?
>
> When and what do you zero?
My use case was just reading these stats as a way to verify that I was
actually hitting certain code paths that increment certain stats.
It was beneficial to be able to reset them to zero just to make it
easier to discern when and by how much they increased.
While it would be trivial to put something together which tracked the
change from some baseline value I don't see any reason to not have the
functionality to reset them back to zero as well.
>
> >
> > The stats are just reset to
> > zero
> > irrespective of what is actually written to the file.
> Sounds good. Setting the val would be as simple, but there shouldn't
> be
> a reason to and it is a debug interface, so we can change it anytime.
>
> >
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > ---
> I have applied it to kvm/queue for the moment, thanks.
Thanks,
Suraj
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry
2016-10-17 13:48 ` Christian Borntraeger
@ 2016-10-18 7:18 ` Suraj Jitindar Singh
0 siblings, 0 replies; 7+ messages in thread
From: Suraj Jitindar Singh @ 2016-10-18 7:18 UTC (permalink / raw)
To: Christian Borntraeger, Radim Krčmář; +Cc: kvm, pbonzini
On Mon, 2016-10-17 at 15:48 +0200, Christian Borntraeger wrote:
> On 10/17/2016 03:40 PM, Radim Krčmář wrote:
> >
> > 2016-10-14 17:02+1100, Suraj Jitindar Singh:
> > >
> > > Various kvm vm and vcpu stats are provided via debugfs entries.
> > > Currently there is no way to reset these stats back to zero for
> > > the system
> > > except by stopping all vms.
> > It is not really resetting, just a special case of current behavior
> > --
> > top-level stats are a sum of all existing VMs/VCPUs, so we would
> > see 0
> > if they all had 0.
> >
> > >
> > > Add the ability to clear (reset back to zero) these stats on a
> > > per stat
> > > basis by writing to the debugfs files.
> > I can see this being used before a test, but the userspace could
> > also
> > read the value instead of writing and subtract it from the value
> > after
> > the test to get a similar result.
> >
> > The userspace-only solution is insufficient?
> >
> > When and what do you zero?
> >
> > >
> > > The stats are just reset
> > > to zero
> > > irrespective of what is actually written to the file.
> > Sounds good. Setting the val would be as simple, but there
> > shouldn't be
> > a reason to and it is a debug interface, so we can change it
> > anytime.
> >
> > >
> > > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> > > ---
> > I have applied it to kvm/queue for the moment, thanks.
> FWIW, I was thinking about another change: provide the accumulated
> values of
> the counters of all VMs that have been destroyed (basically on
> vm_destroy read
> the values and add them on top of the "forever" counters.
> Did not find the time to do something like this.
This doesn't seem like it would be particularly hard to implement, I'm
just not sure of the use case.
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-10-18 7:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-14 6:02 [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry Suraj Jitindar Singh
2016-10-17 13:40 ` Radim Krčmář
2016-10-17 13:48 ` Christian Borntraeger
2016-10-18 7:18 ` Suraj Jitindar Singh
2016-10-18 7:16 ` Suraj Jitindar Singh
2016-10-17 16:50 ` Paolo Bonzini
2016-10-18 6:20 ` Suraj Jitindar Singh
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).