Messages in this thread | | | Date | Sat, 26 Jan 2019 14:53:36 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH 3/6] x86: kvm: no need to check return value of debugfs_create functions |
| |
On Fri, Jan 25, 2019 at 06:49:47PM +0100, Paolo Bonzini wrote: > On 22/01/19 15:35, 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: Thomas Gleixner <tglx@linutronix.de> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: "H. Peter Anvin" <hpa@zytor.com> > > Cc: <x86@kernel.org> > > Cc: <kvm@vger.kernel.org> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > arch/x86/kvm/debugfs.c | 26 ++++++++------------------ > > 1 file changed, 8 insertions(+), 18 deletions(-) > > > > diff --git a/arch/x86/kvm/debugfs.c b/arch/x86/kvm/debugfs.c > > index c19c7ede9bd6..827cd58400d2 100644 > > --- a/arch/x86/kvm/debugfs.c > > +++ b/arch/x86/kvm/debugfs.c > > @@ -43,26 +43,16 @@ DEFINE_SIMPLE_ATTRIBUTE(vcpu_tsc_scaling_frac_fops, vcpu_get_tsc_scaling_frac_bi > > > > int kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu) > > { > > - struct dentry *ret; > > - > > - ret = debugfs_create_file("tsc-offset", 0444, > > - vcpu->debugfs_dentry, > > - vcpu, &vcpu_tsc_offset_fops); > > - if (!ret) > > - return -ENOMEM; > > + debugfs_create_file("tsc-offset", 0444, vcpu->debugfs_dentry, vcpu, > > + &vcpu_tsc_offset_fops); > > > > if (kvm_has_tsc_control) { > > - ret = debugfs_create_file("tsc-scaling-ratio", 0444, > > - vcpu->debugfs_dentry, > > - vcpu, &vcpu_tsc_scaling_fops); > > - if (!ret) > > - return -ENOMEM; > > - ret = debugfs_create_file("tsc-scaling-ratio-frac-bits", 0444, > > - vcpu->debugfs_dentry, > > - vcpu, &vcpu_tsc_scaling_frac_fops); > > - if (!ret) > > - return -ENOMEM; > > - > > + debugfs_create_file("tsc-scaling-ratio", 0444, > > + vcpu->debugfs_dentry, vcpu, > > + &vcpu_tsc_scaling_fops); > > + debugfs_create_file("tsc-scaling-ratio-frac-bits", 0444, > > + vcpu->debugfs_dentry, vcpu, > > + &vcpu_tsc_scaling_frac_fops); > > } > > > > return 0; > > > > I'm still not sure about this. I think it's better if debugfs files are > created "all or none", i.e. something like > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 5ecea812cb6a..ce70c30b2861 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2573,9 +2573,7 @@ static int kvm_vm_ioctl_create_vcpu( > if (r) > goto vcpu_destroy; > > - r = kvm_create_vcpu_debugfs(vcpu); > - if (r) > - goto vcpu_destroy; > + kvm_create_vcpu_debugfs(vcpu);
Ah, yes, sorry, I missed that. I'll respin the patch with this change in it in a few days.
thanks for the review.
greg k-h
| |