Re: [PATCH] kvm/stats: Update kvm stats to clear on write to their debugfs entry

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

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