| Message ID | 20191213130721.7942-1-milanpa@amazon.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5] kvm: Refactor handling of VM debugfs files | expand |
> Am 13.12.2019 um 14:07 schrieb Pandurov, Milan <milanpa@amazon.de>: > > We can store reference to kvm_stats_debugfs_item instead of copying > its values to kvm_stat_data. > This allows us to remove duplicated code and usage of temporary > kvm_stat_data inside vm_stat_get et al. > > Signed-off-by: Milan Pandurov <milanpa@amazon.de> > Reviewed-by: Alexander Graf <graf@anazon.com> If there are no further comments could whoever applies this please fix my typo? :) Thanks, Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On 13.12.19 14:07, Milan Pandurov wrote: > We can store reference to kvm_stats_debugfs_item instead of copying > its values to kvm_stat_data. > This allows us to remove duplicated code and usage of temporary > kvm_stat_data inside vm_stat_get et al. > > Signed-off-by: Milan Pandurov <milanpa@amazon.de> > Reviewed-by: Alexander Graf <graf@anazon.com> > I gave this a quick spin and it still seems to work. > --- > v1 -> v2: > - fix compile issues > - add reference to kvm_stats_debugfs_item in kvm_stat_data > - return -EINVAL when writing !0 > - use explicit switch case instead of ops indirection > - fix checkpatch warning: Change S_IWUGO to 0222 > > v2 -> v3: > - remove unused kvm_stat_ops > - fix style issues > > v3 -> v4: > - revert: Change S_IWUGO to 0222 > > v4 -> v5: > - fix checkpatch warning: Change S_IWUGO to 0222 > --- > include/linux/kvm_host.h | 7 +- > virt/kvm/kvm_main.c | 142 +++++++++++++++++++-------------------- > 2 files changed, 76 insertions(+), 73 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 7ed1e2f8641e..d3f2c0eae857 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1109,9 +1109,8 @@ enum kvm_stat_kind { > }; > > struct kvm_stat_data { > - int offset; > - int mode; > struct kvm *kvm; > + struct kvm_stats_debugfs_item *dbgfs_item; > }; > > struct kvm_stats_debugfs_item { > @@ -1120,6 +1119,10 @@ struct kvm_stats_debugfs_item { > enum kvm_stat_kind kind; > int mode; > }; > + > +#define KVM_DBGFS_GET_MODE(dbgfs_item) \ > + ((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644) > + > extern struct kvm_stats_debugfs_item debugfs_entries[]; > extern struct dentry *kvm_debugfs_dir; > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 00268290dcbd..0ebd6aa95671 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -113,7 +113,7 @@ struct dentry *kvm_debugfs_dir; > EXPORT_SYMBOL_GPL(kvm_debugfs_dir); > > static int kvm_debugfs_num_entries; > -static const struct file_operations *stat_fops_per_vm[]; > +static const struct file_operations stat_fops_per_vm; > > static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, > unsigned long arg); > @@ -650,11 +650,11 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd) > return -ENOMEM; > > stat_data->kvm = kvm; > - stat_data->offset = p->offset; > - stat_data->mode = p->mode ? p->mode : 0644; > + stat_data->dbgfs_item = p; > kvm->debugfs_stat_data[p - debugfs_entries] = stat_data; > - debugfs_create_file(p->name, stat_data->mode, kvm->debugfs_dentry, > - stat_data, stat_fops_per_vm[p->kind]); > + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), > + kvm->debugfs_dentry, stat_data, > + &stat_fops_per_vm); > } > return 0; > } > @@ -4013,8 +4013,9 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file, > return -ENOENT; > > if (simple_attr_open(inode, file, get, > - stat_data->mode & S_IWUGO ? set : NULL, > - fmt)) { > + KVM_DBGFS_GET_MODE(stat_data->dbgfs_item) & 0222 > + ? set : NULL, > + fmt)) { > kvm_put_kvm(stat_data->kvm); > return -ENOMEM; > } > @@ -4033,105 +4034,111 @@ static int kvm_debugfs_release(struct inode *inode, struct file *file) > return 0; > } > > -static int vm_stat_get_per_vm(void *data, u64 *val) > +static int kvm_get_stat_per_vm(struct kvm *kvm, size_t offset, u64 *val) > { > - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > + *val = *(ulong *)((void *)kvm + offset); > > - *val = *(ulong *)((void *)stat_data->kvm + stat_data->offset); > + return 0; > +} > + > +static int kvm_clear_stat_per_vm(struct kvm *kvm, size_t offset) > +{ > + *(ulong *)((void *)kvm + offset) = 0; > > return 0; > } > > -static int vm_stat_clear_per_vm(void *data, u64 val) > +static int kvm_get_stat_per_vcpu(struct kvm *kvm, size_t offset, u64 *val) > { > - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > + int i; > + struct kvm_vcpu *vcpu; > > - if (val) > - return -EINVAL; > + *val = 0; > > - *(ulong *)((void *)stat_data->kvm + stat_data->offset) = 0; > + kvm_for_each_vcpu(i, vcpu, kvm) > + *val += *(u64 *)((void *)vcpu + offset); > > return 0; > } > > -static int vm_stat_get_per_vm_open(struct inode *inode, struct file *file) > +static int kvm_clear_stat_per_vcpu(struct kvm *kvm, size_t offset) > { > - __simple_attr_check_format("%llu\n", 0ull); > - return kvm_debugfs_open(inode, file, vm_stat_get_per_vm, > - vm_stat_clear_per_vm, "%llu\n"); > -} > + int i; > + struct kvm_vcpu *vcpu; > > -static const struct file_operations vm_stat_get_per_vm_fops = { > - .owner = THIS_MODULE, > - .open = vm_stat_get_per_vm_open, > - .release = kvm_debugfs_release, > - .read = simple_attr_read, > - .write = simple_attr_write, > - .llseek = no_llseek, > -}; > + kvm_for_each_vcpu(i, vcpu, kvm) > + *(u64 *)((void *)vcpu + offset) = 0; > + > + return 0; > +} > > -static int vcpu_stat_get_per_vm(void *data, u64 *val) > +static int kvm_stat_data_get(void *data, u64 *val) > { > - int i; > + int r = -EFAULT; > struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > - struct kvm_vcpu *vcpu; > - > - *val = 0; > > - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) > - *val += *(u64 *)((void *)vcpu + stat_data->offset); > + switch (stat_data->dbgfs_item->kind) { > + case KVM_STAT_VM: > + r = kvm_get_stat_per_vm(stat_data->kvm, > + stat_data->dbgfs_item->offset, val); > + break; > + case KVM_STAT_VCPU: > + r = kvm_get_stat_per_vcpu(stat_data->kvm, > + stat_data->dbgfs_item->offset, val); > + break; > + } > > - return 0; > + return r; > } > > -static int vcpu_stat_clear_per_vm(void *data, u64 val) > +static int kvm_stat_data_clear(void *data, u64 val) > { > - int i; > + int r = -EFAULT; > struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > - struct kvm_vcpu *vcpu; > > if (val) > return -EINVAL; > > - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) > - *(u64 *)((void *)vcpu + stat_data->offset) = 0; > + switch (stat_data->dbgfs_item->kind) { > + case KVM_STAT_VM: > + r = kvm_clear_stat_per_vm(stat_data->kvm, > + stat_data->dbgfs_item->offset); > + break; > + case KVM_STAT_VCPU: > + r = kvm_clear_stat_per_vcpu(stat_data->kvm, > + stat_data->dbgfs_item->offset); > + break; > + } > > - return 0; > + return r; > } > > -static int vcpu_stat_get_per_vm_open(struct inode *inode, struct file *file) > +static int kvm_stat_data_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, > - vcpu_stat_clear_per_vm, "%llu\n"); > + return kvm_debugfs_open(inode, file, kvm_stat_data_get, > + kvm_stat_data_clear, "%llu\n"); > } > > -static const struct file_operations vcpu_stat_get_per_vm_fops = { > - .owner = THIS_MODULE, > - .open = vcpu_stat_get_per_vm_open, > +static const struct file_operations stat_fops_per_vm = { > + .owner = THIS_MODULE, > + .open = kvm_stat_data_open, > .release = kvm_debugfs_release, > - .read = simple_attr_read, > - .write = simple_attr_write, > - .llseek = no_llseek, > -}; > - > -static const struct file_operations *stat_fops_per_vm[] = { > - [KVM_STAT_VCPU] = &vcpu_stat_get_per_vm_fops, > - [KVM_STAT_VM] = &vm_stat_get_per_vm_fops, > + .read = simple_attr_read, > + .write = simple_attr_write, > + .llseek = no_llseek, > }; > > static int vm_stat_get(void *_offset, u64 *val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > u64 tmp_val; > > *val = 0; > mutex_lock(&kvm_lock); > list_for_each_entry(kvm, &vm_list, vm_list) { > - stat_tmp.kvm = kvm; > - vm_stat_get_per_vm((void *)&stat_tmp, &tmp_val); > + kvm_get_stat_per_vm(kvm, offset, &tmp_val); > *val += tmp_val; > } > mutex_unlock(&kvm_lock); > @@ -4142,15 +4149,13 @@ static int vm_stat_clear(void *_offset, u64 val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > > if (val) > return -EINVAL; > > mutex_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); > + kvm_clear_stat_per_vm(kvm, offset); > } > mutex_unlock(&kvm_lock); > > @@ -4163,14 +4168,12 @@ static int vcpu_stat_get(void *_offset, u64 *val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > u64 tmp_val; > > *val = 0; > mutex_lock(&kvm_lock); > list_for_each_entry(kvm, &vm_list, vm_list) { > - stat_tmp.kvm = kvm; > - vcpu_stat_get_per_vm((void *)&stat_tmp, &tmp_val); > + kvm_get_stat_per_vcpu(kvm, offset, &tmp_val); > *val += tmp_val; > } > mutex_unlock(&kvm_lock); > @@ -4181,15 +4184,13 @@ static int vcpu_stat_clear(void *_offset, u64 val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > > if (val) > return -EINVAL; > > mutex_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); > + kvm_clear_stat_per_vcpu(kvm, offset); > } > mutex_unlock(&kvm_lock); > > @@ -4262,9 +4263,8 @@ static void kvm_init_debug(void) > > kvm_debugfs_num_entries = 0; > for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) { > - int mode = p->mode ? p->mode : 0644; > - debugfs_create_file(p->name, mode, kvm_debugfs_dir, > - (void *)(long)p->offset, > + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), > + kvm_debugfs_dir, (void *)(long)p->offset, > stat_fops[p->kind]); > } > } >
On 13/12/19 14:07, Milan Pandurov wrote: > We can store reference to kvm_stats_debugfs_item instead of copying > its values to kvm_stat_data. > This allows us to remove duplicated code and usage of temporary > kvm_stat_data inside vm_stat_get et al. > > Signed-off-by: Milan Pandurov <milanpa@amazon.de> > Reviewed-by: Alexander Graf <graf@anazon.com> > > --- > v1 -> v2: > - fix compile issues > - add reference to kvm_stats_debugfs_item in kvm_stat_data > - return -EINVAL when writing !0 > - use explicit switch case instead of ops indirection > - fix checkpatch warning: Change S_IWUGO to 0222 > > v2 -> v3: > - remove unused kvm_stat_ops > - fix style issues > > v3 -> v4: > - revert: Change S_IWUGO to 0222 > > v4 -> v5: > - fix checkpatch warning: Change S_IWUGO to 0222 > --- > include/linux/kvm_host.h | 7 +- > virt/kvm/kvm_main.c | 142 +++++++++++++++++++-------------------- > 2 files changed, 76 insertions(+), 73 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 7ed1e2f8641e..d3f2c0eae857 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -1109,9 +1109,8 @@ enum kvm_stat_kind { > }; > > struct kvm_stat_data { > - int offset; > - int mode; > struct kvm *kvm; > + struct kvm_stats_debugfs_item *dbgfs_item; > }; > > struct kvm_stats_debugfs_item { > @@ -1120,6 +1119,10 @@ struct kvm_stats_debugfs_item { > enum kvm_stat_kind kind; > int mode; > }; > + > +#define KVM_DBGFS_GET_MODE(dbgfs_item) \ > + ((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644) > + > extern struct kvm_stats_debugfs_item debugfs_entries[]; > extern struct dentry *kvm_debugfs_dir; > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 00268290dcbd..0ebd6aa95671 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -113,7 +113,7 @@ struct dentry *kvm_debugfs_dir; > EXPORT_SYMBOL_GPL(kvm_debugfs_dir); > > static int kvm_debugfs_num_entries; > -static const struct file_operations *stat_fops_per_vm[]; > +static const struct file_operations stat_fops_per_vm; > > static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, > unsigned long arg); > @@ -650,11 +650,11 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd) > return -ENOMEM; > > stat_data->kvm = kvm; > - stat_data->offset = p->offset; > - stat_data->mode = p->mode ? p->mode : 0644; > + stat_data->dbgfs_item = p; > kvm->debugfs_stat_data[p - debugfs_entries] = stat_data; > - debugfs_create_file(p->name, stat_data->mode, kvm->debugfs_dentry, > - stat_data, stat_fops_per_vm[p->kind]); > + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), > + kvm->debugfs_dentry, stat_data, > + &stat_fops_per_vm); > } > return 0; > } > @@ -4013,8 +4013,9 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file, > return -ENOENT; > > if (simple_attr_open(inode, file, get, > - stat_data->mode & S_IWUGO ? set : NULL, > - fmt)) { > + KVM_DBGFS_GET_MODE(stat_data->dbgfs_item) & 0222 > + ? set : NULL, > + fmt)) { > kvm_put_kvm(stat_data->kvm); > return -ENOMEM; > } > @@ -4033,105 +4034,111 @@ static int kvm_debugfs_release(struct inode *inode, struct file *file) > return 0; > } > > -static int vm_stat_get_per_vm(void *data, u64 *val) > +static int kvm_get_stat_per_vm(struct kvm *kvm, size_t offset, u64 *val) > { > - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > + *val = *(ulong *)((void *)kvm + offset); > > - *val = *(ulong *)((void *)stat_data->kvm + stat_data->offset); > + return 0; > +} > + > +static int kvm_clear_stat_per_vm(struct kvm *kvm, size_t offset) > +{ > + *(ulong *)((void *)kvm + offset) = 0; > > return 0; > } > > -static int vm_stat_clear_per_vm(void *data, u64 val) > +static int kvm_get_stat_per_vcpu(struct kvm *kvm, size_t offset, u64 *val) > { > - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > + int i; > + struct kvm_vcpu *vcpu; > > - if (val) > - return -EINVAL; > + *val = 0; > > - *(ulong *)((void *)stat_data->kvm + stat_data->offset) = 0; > + kvm_for_each_vcpu(i, vcpu, kvm) > + *val += *(u64 *)((void *)vcpu + offset); > > return 0; > } > > -static int vm_stat_get_per_vm_open(struct inode *inode, struct file *file) > +static int kvm_clear_stat_per_vcpu(struct kvm *kvm, size_t offset) > { > - __simple_attr_check_format("%llu\n", 0ull); > - return kvm_debugfs_open(inode, file, vm_stat_get_per_vm, > - vm_stat_clear_per_vm, "%llu\n"); > -} > + int i; > + struct kvm_vcpu *vcpu; > > -static const struct file_operations vm_stat_get_per_vm_fops = { > - .owner = THIS_MODULE, > - .open = vm_stat_get_per_vm_open, > - .release = kvm_debugfs_release, > - .read = simple_attr_read, > - .write = simple_attr_write, > - .llseek = no_llseek, > -}; > + kvm_for_each_vcpu(i, vcpu, kvm) > + *(u64 *)((void *)vcpu + offset) = 0; > + > + return 0; > +} > > -static int vcpu_stat_get_per_vm(void *data, u64 *val) > +static int kvm_stat_data_get(void *data, u64 *val) > { > - int i; > + int r = -EFAULT; > struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > - struct kvm_vcpu *vcpu; > - > - *val = 0; > > - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) > - *val += *(u64 *)((void *)vcpu + stat_data->offset); > + switch (stat_data->dbgfs_item->kind) { > + case KVM_STAT_VM: > + r = kvm_get_stat_per_vm(stat_data->kvm, > + stat_data->dbgfs_item->offset, val); > + break; > + case KVM_STAT_VCPU: > + r = kvm_get_stat_per_vcpu(stat_data->kvm, > + stat_data->dbgfs_item->offset, val); > + break; > + } > > - return 0; > + return r; > } > > -static int vcpu_stat_clear_per_vm(void *data, u64 val) > +static int kvm_stat_data_clear(void *data, u64 val) > { > - int i; > + int r = -EFAULT; > struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; > - struct kvm_vcpu *vcpu; > > if (val) > return -EINVAL; > > - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) > - *(u64 *)((void *)vcpu + stat_data->offset) = 0; > + switch (stat_data->dbgfs_item->kind) { > + case KVM_STAT_VM: > + r = kvm_clear_stat_per_vm(stat_data->kvm, > + stat_data->dbgfs_item->offset); > + break; > + case KVM_STAT_VCPU: > + r = kvm_clear_stat_per_vcpu(stat_data->kvm, > + stat_data->dbgfs_item->offset); > + break; > + } > > - return 0; > + return r; > } > > -static int vcpu_stat_get_per_vm_open(struct inode *inode, struct file *file) > +static int kvm_stat_data_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, > - vcpu_stat_clear_per_vm, "%llu\n"); > + return kvm_debugfs_open(inode, file, kvm_stat_data_get, > + kvm_stat_data_clear, "%llu\n"); > } > > -static const struct file_operations vcpu_stat_get_per_vm_fops = { > - .owner = THIS_MODULE, > - .open = vcpu_stat_get_per_vm_open, > +static const struct file_operations stat_fops_per_vm = { > + .owner = THIS_MODULE, > + .open = kvm_stat_data_open, > .release = kvm_debugfs_release, > - .read = simple_attr_read, > - .write = simple_attr_write, > - .llseek = no_llseek, > -}; > - > -static const struct file_operations *stat_fops_per_vm[] = { > - [KVM_STAT_VCPU] = &vcpu_stat_get_per_vm_fops, > - [KVM_STAT_VM] = &vm_stat_get_per_vm_fops, > + .read = simple_attr_read, > + .write = simple_attr_write, > + .llseek = no_llseek, > }; > > static int vm_stat_get(void *_offset, u64 *val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > u64 tmp_val; > > *val = 0; > mutex_lock(&kvm_lock); > list_for_each_entry(kvm, &vm_list, vm_list) { > - stat_tmp.kvm = kvm; > - vm_stat_get_per_vm((void *)&stat_tmp, &tmp_val); > + kvm_get_stat_per_vm(kvm, offset, &tmp_val); > *val += tmp_val; > } > mutex_unlock(&kvm_lock); > @@ -4142,15 +4149,13 @@ static int vm_stat_clear(void *_offset, u64 val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > > if (val) > return -EINVAL; > > mutex_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); > + kvm_clear_stat_per_vm(kvm, offset); > } > mutex_unlock(&kvm_lock); > > @@ -4163,14 +4168,12 @@ static int vcpu_stat_get(void *_offset, u64 *val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > u64 tmp_val; > > *val = 0; > mutex_lock(&kvm_lock); > list_for_each_entry(kvm, &vm_list, vm_list) { > - stat_tmp.kvm = kvm; > - vcpu_stat_get_per_vm((void *)&stat_tmp, &tmp_val); > + kvm_get_stat_per_vcpu(kvm, offset, &tmp_val); > *val += tmp_val; > } > mutex_unlock(&kvm_lock); > @@ -4181,15 +4184,13 @@ static int vcpu_stat_clear(void *_offset, u64 val) > { > unsigned offset = (long)_offset; > struct kvm *kvm; > - struct kvm_stat_data stat_tmp = {.offset = offset}; > > if (val) > return -EINVAL; > > mutex_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); > + kvm_clear_stat_per_vcpu(kvm, offset); > } > mutex_unlock(&kvm_lock); > > @@ -4262,9 +4263,8 @@ static void kvm_init_debug(void) > > kvm_debugfs_num_entries = 0; > for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) { > - int mode = p->mode ? p->mode : 0644; > - debugfs_create_file(p->name, mode, kvm_debugfs_dir, > - (void *)(long)p->offset, > + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), > + kvm_debugfs_dir, (void *)(long)p->offset, > stat_fops[p->kind]); > } > } > Queued, thnaks. Sorry for the delay. Paolo
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 7ed1e2f8641e..d3f2c0eae857 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -1109,9 +1109,8 @@ enum kvm_stat_kind { }; struct kvm_stat_data { - int offset; - int mode; struct kvm *kvm; + struct kvm_stats_debugfs_item *dbgfs_item; }; struct kvm_stats_debugfs_item { @@ -1120,6 +1119,10 @@ struct kvm_stats_debugfs_item { enum kvm_stat_kind kind; int mode; }; + +#define KVM_DBGFS_GET_MODE(dbgfs_item) \ + ((dbgfs_item)->mode ? (dbgfs_item)->mode : 0644) + extern struct kvm_stats_debugfs_item debugfs_entries[]; extern struct dentry *kvm_debugfs_dir; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 00268290dcbd..0ebd6aa95671 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -113,7 +113,7 @@ struct dentry *kvm_debugfs_dir; EXPORT_SYMBOL_GPL(kvm_debugfs_dir); static int kvm_debugfs_num_entries; -static const struct file_operations *stat_fops_per_vm[]; +static const struct file_operations stat_fops_per_vm; static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl, unsigned long arg); @@ -650,11 +650,11 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd) return -ENOMEM; stat_data->kvm = kvm; - stat_data->offset = p->offset; - stat_data->mode = p->mode ? p->mode : 0644; + stat_data->dbgfs_item = p; kvm->debugfs_stat_data[p - debugfs_entries] = stat_data; - debugfs_create_file(p->name, stat_data->mode, kvm->debugfs_dentry, - stat_data, stat_fops_per_vm[p->kind]); + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), + kvm->debugfs_dentry, stat_data, + &stat_fops_per_vm); } return 0; } @@ -4013,8 +4013,9 @@ static int kvm_debugfs_open(struct inode *inode, struct file *file, return -ENOENT; if (simple_attr_open(inode, file, get, - stat_data->mode & S_IWUGO ? set : NULL, - fmt)) { + KVM_DBGFS_GET_MODE(stat_data->dbgfs_item) & 0222 + ? set : NULL, + fmt)) { kvm_put_kvm(stat_data->kvm); return -ENOMEM; } @@ -4033,105 +4034,111 @@ static int kvm_debugfs_release(struct inode *inode, struct file *file) return 0; } -static int vm_stat_get_per_vm(void *data, u64 *val) +static int kvm_get_stat_per_vm(struct kvm *kvm, size_t offset, u64 *val) { - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; + *val = *(ulong *)((void *)kvm + offset); - *val = *(ulong *)((void *)stat_data->kvm + stat_data->offset); + return 0; +} + +static int kvm_clear_stat_per_vm(struct kvm *kvm, size_t offset) +{ + *(ulong *)((void *)kvm + offset) = 0; return 0; } -static int vm_stat_clear_per_vm(void *data, u64 val) +static int kvm_get_stat_per_vcpu(struct kvm *kvm, size_t offset, u64 *val) { - struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; + int i; + struct kvm_vcpu *vcpu; - if (val) - return -EINVAL; + *val = 0; - *(ulong *)((void *)stat_data->kvm + stat_data->offset) = 0; + kvm_for_each_vcpu(i, vcpu, kvm) + *val += *(u64 *)((void *)vcpu + offset); return 0; } -static int vm_stat_get_per_vm_open(struct inode *inode, struct file *file) +static int kvm_clear_stat_per_vcpu(struct kvm *kvm, size_t offset) { - __simple_attr_check_format("%llu\n", 0ull); - return kvm_debugfs_open(inode, file, vm_stat_get_per_vm, - vm_stat_clear_per_vm, "%llu\n"); -} + int i; + struct kvm_vcpu *vcpu; -static const struct file_operations vm_stat_get_per_vm_fops = { - .owner = THIS_MODULE, - .open = vm_stat_get_per_vm_open, - .release = kvm_debugfs_release, - .read = simple_attr_read, - .write = simple_attr_write, - .llseek = no_llseek, -}; + kvm_for_each_vcpu(i, vcpu, kvm) + *(u64 *)((void *)vcpu + offset) = 0; + + return 0; +} -static int vcpu_stat_get_per_vm(void *data, u64 *val) +static int kvm_stat_data_get(void *data, u64 *val) { - int i; + int r = -EFAULT; struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; - struct kvm_vcpu *vcpu; - - *val = 0; - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) - *val += *(u64 *)((void *)vcpu + stat_data->offset); + switch (stat_data->dbgfs_item->kind) { + case KVM_STAT_VM: + r = kvm_get_stat_per_vm(stat_data->kvm, + stat_data->dbgfs_item->offset, val); + break; + case KVM_STAT_VCPU: + r = kvm_get_stat_per_vcpu(stat_data->kvm, + stat_data->dbgfs_item->offset, val); + break; + } - return 0; + return r; } -static int vcpu_stat_clear_per_vm(void *data, u64 val) +static int kvm_stat_data_clear(void *data, u64 val) { - int i; + int r = -EFAULT; struct kvm_stat_data *stat_data = (struct kvm_stat_data *)data; - struct kvm_vcpu *vcpu; if (val) return -EINVAL; - kvm_for_each_vcpu(i, vcpu, stat_data->kvm) - *(u64 *)((void *)vcpu + stat_data->offset) = 0; + switch (stat_data->dbgfs_item->kind) { + case KVM_STAT_VM: + r = kvm_clear_stat_per_vm(stat_data->kvm, + stat_data->dbgfs_item->offset); + break; + case KVM_STAT_VCPU: + r = kvm_clear_stat_per_vcpu(stat_data->kvm, + stat_data->dbgfs_item->offset); + break; + } - return 0; + return r; } -static int vcpu_stat_get_per_vm_open(struct inode *inode, struct file *file) +static int kvm_stat_data_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, - vcpu_stat_clear_per_vm, "%llu\n"); + return kvm_debugfs_open(inode, file, kvm_stat_data_get, + kvm_stat_data_clear, "%llu\n"); } -static const struct file_operations vcpu_stat_get_per_vm_fops = { - .owner = THIS_MODULE, - .open = vcpu_stat_get_per_vm_open, +static const struct file_operations stat_fops_per_vm = { + .owner = THIS_MODULE, + .open = kvm_stat_data_open, .release = kvm_debugfs_release, - .read = simple_attr_read, - .write = simple_attr_write, - .llseek = no_llseek, -}; - -static const struct file_operations *stat_fops_per_vm[] = { - [KVM_STAT_VCPU] = &vcpu_stat_get_per_vm_fops, - [KVM_STAT_VM] = &vm_stat_get_per_vm_fops, + .read = simple_attr_read, + .write = simple_attr_write, + .llseek = no_llseek, }; static int vm_stat_get(void *_offset, u64 *val) { unsigned offset = (long)_offset; struct kvm *kvm; - struct kvm_stat_data stat_tmp = {.offset = offset}; u64 tmp_val; *val = 0; mutex_lock(&kvm_lock); list_for_each_entry(kvm, &vm_list, vm_list) { - stat_tmp.kvm = kvm; - vm_stat_get_per_vm((void *)&stat_tmp, &tmp_val); + kvm_get_stat_per_vm(kvm, offset, &tmp_val); *val += tmp_val; } mutex_unlock(&kvm_lock); @@ -4142,15 +4149,13 @@ static int vm_stat_clear(void *_offset, u64 val) { unsigned offset = (long)_offset; struct kvm *kvm; - struct kvm_stat_data stat_tmp = {.offset = offset}; if (val) return -EINVAL; mutex_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); + kvm_clear_stat_per_vm(kvm, offset); } mutex_unlock(&kvm_lock); @@ -4163,14 +4168,12 @@ static int vcpu_stat_get(void *_offset, u64 *val) { unsigned offset = (long)_offset; struct kvm *kvm; - struct kvm_stat_data stat_tmp = {.offset = offset}; u64 tmp_val; *val = 0; mutex_lock(&kvm_lock); list_for_each_entry(kvm, &vm_list, vm_list) { - stat_tmp.kvm = kvm; - vcpu_stat_get_per_vm((void *)&stat_tmp, &tmp_val); + kvm_get_stat_per_vcpu(kvm, offset, &tmp_val); *val += tmp_val; } mutex_unlock(&kvm_lock); @@ -4181,15 +4184,13 @@ static int vcpu_stat_clear(void *_offset, u64 val) { unsigned offset = (long)_offset; struct kvm *kvm; - struct kvm_stat_data stat_tmp = {.offset = offset}; if (val) return -EINVAL; mutex_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); + kvm_clear_stat_per_vcpu(kvm, offset); } mutex_unlock(&kvm_lock); @@ -4262,9 +4263,8 @@ static void kvm_init_debug(void) kvm_debugfs_num_entries = 0; for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) { - int mode = p->mode ? p->mode : 0644; - debugfs_create_file(p->name, mode, kvm_debugfs_dir, - (void *)(long)p->offset, + debugfs_create_file(p->name, KVM_DBGFS_GET_MODE(p), + kvm_debugfs_dir, (void *)(long)p->offset, stat_fops[p->kind]); } }