[PATCH] kvm: no need to check return value of debugfs_create functions

[PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 virt/kvm/kvm_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5ecea812cb6a..4f96450ecdfc 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2528,9 +2528,7 @@ static int kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
 
 	snprintf(dir_name, sizeof(dir_name), "vcpu%d", vcpu->vcpu_id);
 	vcpu->debugfs_dentry = debugfs_create_dir(dir_name,
-								vcpu->kvm->debugfs_dentry);
-	if (!vcpu->debugfs_dentry)
-		return -ENOMEM;
+						  vcpu->kvm->debugfs_dentry);
 
 	ret = kvm_arch_create_vcpu_debugfs(vcpu);
 	if (ret < 0) {
-- 
2.20.1

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Sean Christopherson]

On Tue, Jan 22, 2019 at 04:21:50PM +0100, 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.

What about wanting to make the debugfs all-or-nothing?  That seems like
a legitimate usage of checking the return value.

E.g. KVM removes the debugfs if kvm_arch_create_vcpu_debugfs() fails, and
the arch/x86/kvm/debugfs.c implementation of kvm_arch_create_vcpu_debugfs()
returns an error if any of its debugfs_create_file() calls fail.

If you're adamant about removing all debugfs create return value checks,
the aforementioned debugfs_create_file() calls should also be removed.
And at that point kvm_create_vcpu_debugfs() should have a 'void' return
value.

> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  virt/kvm/kvm_main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5ecea812cb6a..4f96450ecdfc 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2528,9 +2528,7 @@ static int kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
>  
>  	snprintf(dir_name, sizeof(dir_name), "vcpu%d", vcpu->vcpu_id);
>  	vcpu->debugfs_dentry = debugfs_create_dir(dir_name,
> -								vcpu->kvm->debugfs_dentry);
> -	if (!vcpu->debugfs_dentry)
> -		return -ENOMEM;
> +						  vcpu->kvm->debugfs_dentry);
>  
>  	ret = kvm_arch_create_vcpu_debugfs(vcpu);
>  	if (ret < 0) {
> -- 
> 2.20.1
> 

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Sean Christopherson]

On Tue, Jan 22, 2019 at 09:21:02AM -0800, Sean Christopherson wrote:
> On Tue, Jan 22, 2019 at 04:21:50PM +0100, 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.
> 
> What about wanting to make the debugfs all-or-nothing?  That seems like
> a legitimate usage of checking the return value.
> 
> E.g. KVM removes the debugfs if kvm_arch_create_vcpu_debugfs() fails, and
> the arch/x86/kvm/debugfs.c implementation of kvm_arch_create_vcpu_debugfs()
> returns an error if any of its debugfs_create_file() calls fail.
> 
> If you're adamant about removing all debugfs create return value checks,
> the aforementioned debugfs_create_file() calls should also be removed.
> And at that point kvm_create_vcpu_debugfs() should have a 'void' return
> value.

Belatedly saw the other series.  It'll require a bit more coordination,
but folding this into the x86 series would allow for converting the KVM
call stack to have 'void' returns.

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Tue, Jan 22, 2019 at 09:29:07AM -0800, Sean Christopherson wrote:
> On Tue, Jan 22, 2019 at 09:21:02AM -0800, Sean Christopherson wrote:
> > On Tue, Jan 22, 2019 at 04:21:50PM +0100, 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.
> > 
> > What about wanting to make the debugfs all-or-nothing?  That seems like
> > a legitimate usage of checking the return value.
> > 
> > E.g. KVM removes the debugfs if kvm_arch_create_vcpu_debugfs() fails, and
> > the arch/x86/kvm/debugfs.c implementation of kvm_arch_create_vcpu_debugfs()
> > returns an error if any of its debugfs_create_file() calls fail.
> > 
> > If you're adamant about removing all debugfs create return value checks,
> > the aforementioned debugfs_create_file() calls should also be removed.
> > And at that point kvm_create_vcpu_debugfs() should have a 'void' return
> > value.
> 
> Belatedly saw the other series.  It'll require a bit more coordination,
> but folding this into the x86 series would allow for converting the KVM
> call stack to have 'void' returns.

Oh, nice, want me to tack this onto the end of there, or just do some
follow-on patches after this gets merged?

thanks,

greg k-h

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Christian Borntraeger]

On 22.01.2019 16:21, 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.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  virt/kvm/kvm_main.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5ecea812cb6a..4f96450ecdfc 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2528,9 +2528,7 @@ static int kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> 
>  	snprintf(dir_name, sizeof(dir_name), "vcpu%d", vcpu->vcpu_id);
>  	vcpu->debugfs_dentry = debugfs_create_dir(dir_name,
> -								vcpu->kvm->debugfs_dentry);
> -	if (!vcpu->debugfs_dentry)
> -		return -ENOMEM;
> +						  vcpu->kvm->debugfs_dentry);
> 
>  	ret = kvm_arch_create_vcpu_debugfs(vcpu);
>  	if (ret < 0) {
> 


The interesting part of these debugfs entries is that they export an interface that is used
by the kvm_stat tool. (and all distributions that I checked have debugfs enabled).

I think it is pretty unlikely that things will fail, but the question is: do we want to reject
VM creation if that VM cannot be observed by instrumentation or not? No idea.

This also brings the question: shall we move these counters out of debugfs into something else?

Christian

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Tue, Jan 22, 2019 at 09:39:24PM +0100, Christian Borntraeger wrote:
> 
> 
> On 22.01.2019 16:21, 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.
> > 
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> > Cc: kvm@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  virt/kvm/kvm_main.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 5ecea812cb6a..4f96450ecdfc 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2528,9 +2528,7 @@ static int kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> > 
> >  	snprintf(dir_name, sizeof(dir_name), "vcpu%d", vcpu->vcpu_id);
> >  	vcpu->debugfs_dentry = debugfs_create_dir(dir_name,
> > -								vcpu->kvm->debugfs_dentry);
> > -	if (!vcpu->debugfs_dentry)
> > -		return -ENOMEM;
> > +						  vcpu->kvm->debugfs_dentry);
> > 
> >  	ret = kvm_arch_create_vcpu_debugfs(vcpu);
> >  	if (ret < 0) {
> > 
> 
> 
> The interesting part of these debugfs entries is that they export an interface that is used
> by the kvm_stat tool. (and all distributions that I checked have debugfs enabled).
> 
> I think it is pretty unlikely that things will fail, but the question is: do we want to reject
> VM creation if that VM cannot be observed by instrumentation or not? No idea.

No you should not as any other part of the kernel can randomly create
the same debugfs files and keep your code from being able to create
them :)

> This also brings the question: shall we move these counters out of debugfs into something else?

If you have code that relies on debugfs, yes, you need to move that out
of debugfs because more and more systems are trying to disable it due to
the obvious problems with it (i.e. leaking tons of debugging
information).

debugfs is for DEBUG information, not for "statistics about how my VM is
working".  That sounds like something you need to rely on, so debugfs is
not the place for it.

thanks,

greg k-h

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Paolo Bonzini]

On 22/01/19 21:48, Greg Kroah-Hartman wrote:
>> This also brings the question: shall we move these counters out of debugfs into something else?
> If you have code that relies on debugfs, yes, you need to move that out
> of debugfs because more and more systems are trying to disable it due to
> the obvious problems with it (i.e. leaking tons of debugging
> information).
> 
> debugfs is for DEBUG information, not for "statistics about how my VM is
> working".  That sounds like something you need to rely on, so debugfs is
> not the place for it.

Yes, we know that and tracepoints are already one replacement.  However,
they are slower that just a lock-free "vcpu->stats.foo_happened++".
Another idea that Steven Rostedt and I discussed a while ago is some
kind of "statfs" which would already provide some code, similar to the
one that KVM uses to accumulate statistics from multiple VMs or multiple
VCPUs into a single counter.

Paolo

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Christian Borntraeger]

On 23.01.2019 00:11, Paolo Bonzini wrote:
> On 22/01/19 21:48, Greg Kroah-Hartman wrote:
>>> This also brings the question: shall we move these counters out of debugfs into something else?
>> If you have code that relies on debugfs, yes, you need to move that out
>> of debugfs because more and more systems are trying to disable it due to
>> the obvious problems with it (i.e. leaking tons of debugging
>> information).
>>
>> debugfs is for DEBUG information, not for "statistics about how my VM is
>> working".  That sounds like something you need to rely on, so debugfs is
>> not the place for it.
> 
> Yes, we know that and tracepoints are already one replacement.  However,
> they are slower that just a lock-free "vcpu->stats.foo_happened++".

Yes, the tracepoints are not a proper replacement for the counters (especially 
the capability to get numbers after-the-fact. So I would really like to keep
both.

> Another idea that Steven Rostedt and I discussed a while ago is some
> kind of "statfs" which would already provide some code, similar to the
> one that KVM uses to accumulate statistics from multiple VMs or multiple
> VCPUs into a single counter.


I think that would make a lot of sense to have a common filesystem to avoid
code duplication bugs.

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Paolo Bonzini]

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@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: kvmarm@lists.cs.columbia.edu
> Cc: kvm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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

[PATCH] kvm: no need to check return value of debugfs_create functions

[Paolo Bonzini]

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@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Kr?m??" <rkrcmar@redhat.com>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: kvm-ppc at vger.kernel.org
> Cc: linuxppc-dev at lists.ozlabs.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: kvmarm at lists.cs.columbia.edu
> Cc: kvm at vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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

Re: [PATCH] kvm: no need to check return value of debugfs_create functions

[Paolo Bonzini]

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@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: kvmarm@lists.cs.columbia.edu
> Cc: kvm@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  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

[PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: kvmarm@lists.cs.columbia.edu
Cc: kvm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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:
-- 
2.17.0

[PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Kr?m??" <rkrcmar@redhat.com>
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: kvm-ppc at vger.kernel.org
Cc: linuxppc-dev at lists.ozlabs.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: kvmarm at lists.cs.columbia.edu
Cc: kvm at vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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:
-- 
2.17.0

[PATCH] kvm: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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@ozlabs.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Arvind Yadav <arvind.yadav.cs@gmail.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: kvm-ppc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: kvmarm@lists.cs.columbia.edu
Cc: kvm@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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:
-- 
2.17.0