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

[Paolo Bonzini <pbonzini@redhat.com>]

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