Re: [PATCH] kvm: no need to check return value of debugfs_create functions
From: Paolo Bonzini
Date: Tue May 29 2018 - 13:06:40 EST
On 29/05/2018 18:22, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> This cleans up the error handling a lot, as this code will never get
> hit.
>
> Cc: Paul Mackerras <paulus@xxxxxxxxxx>
> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> Cc: Christoffer Dall <christoffer.dall@xxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: "Radim KrÄmÃÅ" <rkrcmar@xxxxxxxxxx>
> Cc: Arvind Yadav <arvind.yadav.cs@xxxxxxxxx>
> Cc: Eric Auger <eric.auger@xxxxxxxxxx>
> Cc: Andre Przywara <andre.przywara@xxxxxxx>
> Cc: kvm-ppc@xxxxxxxxxxxxxxx
> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: kvmarm@xxxxxxxxxxxxxxxxxxxxx
> Cc: kvm@xxxxxxxxxxxxxxx
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> arch/powerpc/kvm/book3s_hv.c | 3 +--
> virt/kvm/arm/vgic/vgic-debug.c | 17 ++++-----------
> virt/kvm/arm/vgic/vgic.h | 4 ++--
> virt/kvm/kvm_main.c | 40 +++++++---------------------------
> 4 files changed, 15 insertions(+), 49 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 4d07fca5121c..67d7de1470cc 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3950,8 +3950,7 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
> */
> snprintf(buf, sizeof(buf), "vm%d", current->pid);
> kvm->arch.debugfs_dir = debugfs_create_dir(buf, kvm_debugfs_dir);
> - if (!IS_ERR_OR_NULL(kvm->arch.debugfs_dir))
> - kvmppc_mmu_debugfs_init(kvm);
> + kvmppc_mmu_debugfs_init(kvm);
>
> return 0;
> }
> diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c
> index 10b38178cff2..0140b29079b6 100644
> --- a/virt/kvm/arm/vgic/vgic-debug.c
> +++ b/virt/kvm/arm/vgic/vgic-debug.c
> @@ -263,21 +263,12 @@ static const struct file_operations vgic_debug_fops = {
> .release = seq_release
> };
>
> -int vgic_debug_init(struct kvm *kvm)
> +void vgic_debug_init(struct kvm *kvm)
> {
> - if (!kvm->debugfs_dentry)
> - return -ENOENT;
> -
> - if (!debugfs_create_file("vgic-state", 0444,
> - kvm->debugfs_dentry,
> - kvm,
> - &vgic_debug_fops))
> - return -ENOMEM;
> -
> - return 0;
> + debugfs_create_file("vgic-state", 0444, kvm->debugfs_dentry, kvm,
> + &vgic_debug_fops);
> }
>
> -int vgic_debug_destroy(struct kvm *kvm)
> +void vgic_debug_destroy(struct kvm *kvm)
> {
> - return 0;
> }
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 830e815748a0..3c38c5349953 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -229,8 +229,8 @@ void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
> int vgic_lazy_init(struct kvm *kvm);
> int vgic_init(struct kvm *kvm);
>
> -int vgic_debug_init(struct kvm *kvm);
> -int vgic_debug_destroy(struct kvm *kvm);
> +void vgic_debug_init(struct kvm *kvm);
> +void vgic_debug_destroy(struct kvm *kvm);
>
> bool lock_all_vcpus(struct kvm *kvm);
> void unlock_all_vcpus(struct kvm *kvm);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index c7b2e927f699..0ad400f353fc 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -572,10 +572,7 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
> return 0;
>
> snprintf(dir_name, sizeof(dir_name), "%d-%d", task_pid_nr(current), fd);
> - kvm->debugfs_dentry = debugfs_create_dir(dir_name,
> - kvm_debugfs_dir);
> - if (!kvm->debugfs_dentry)
> - return -ENOMEM;
> + kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
>
> kvm->debugfs_stat_data = kcalloc(kvm_debugfs_num_entries,
> sizeof(*kvm->debugfs_stat_data),
> @@ -591,11 +588,8 @@ 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, 0644,
> - kvm->debugfs_dentry,
> - stat_data,
> - stat_fops_per_vm[p->kind]))
> - return -ENOMEM;
> + debugfs_create_file(p->name, 0644, kvm->debugfs_dentry,
> + stat_data, stat_fops_per_vm[p->kind]);
> }
> return 0;
> }
> @@ -3896,29 +3890,18 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
> kfree(env);
> }
>
> -static int kvm_init_debug(void)
> +static void kvm_init_debug(void)
> {
> - int r = -EEXIST;
> struct kvm_stats_debugfs_item *p;
>
> kvm_debugfs_dir = debugfs_create_dir("kvm", NULL);
> - if (kvm_debugfs_dir == NULL)
> - goto out;
>
> kvm_debugfs_num_entries = 0;
> for (p = debugfs_entries; p->name; ++p, kvm_debugfs_num_entries++) {
> - if (!debugfs_create_file(p->name, 0644, kvm_debugfs_dir,
> - (void *)(long)p->offset,
> - stat_fops[p->kind]))
> - goto out_dir;
> + debugfs_create_file(p->name, 0644, kvm_debugfs_dir,
> + (void *)(long)p->offset,
> + stat_fops[p->kind]);
> }
> -
> - return 0;
> -
> -out_dir:
> - debugfs_remove_recursive(kvm_debugfs_dir);
> -out:
> - return r;
> }
>
> static int kvm_suspend(void)
> @@ -4046,20 +4029,13 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
> kvm_preempt_ops.sched_in = kvm_sched_in;
> kvm_preempt_ops.sched_out = kvm_sched_out;
>
> - r = kvm_init_debug();
> - if (r) {
> - pr_err("kvm: create debugfs files failed\n");
> - goto out_undebugfs;
> - }
> + kvm_init_debug();
>
> r = kvm_vfio_ops_init();
> WARN_ON(r);
>
> return 0;
>
> -out_undebugfs:
> - unregister_syscore_ops(&kvm_syscore_ops);
> - misc_deregister(&kvm_dev);
> out_unreg:
> kvm_async_pf_deinit();
> out_free:
>
Queued, thanks.
Paolo