[PATCH] kvm: remove invalid check for debugfs_create_dir()

[PATCH] kvm: remove invalid check for debugfs_create_dir()

[Greg Kroah-Hartman]

debugfs_create_dir() can never return NULL, so no need to check for an
impossible thing.

It's also not needed to ever check the return value of this function, so
just remove the check entirely, and indent the previous line to a sane
formatting :)

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 ca54b09adf5b..4b4ef642d8fa 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2605,9 +2605,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.22.0

Re: [PATCH] kvm: remove invalid check for debugfs_create_dir()

[Sean Christopherson]

On Wed, Jun 12, 2019 at 04:50:33PM +0200, Greg Kroah-Hartman wrote:
> debugfs_create_dir() can never return NULL, so no need to check for an
> impossible thing.
> 
> It's also not needed to ever check the return value of this function, so
> just remove the check entirely, and indent the previous line to a sane
> formatting :)
> 
> 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 ca54b09adf5b..4b4ef642d8fa 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2605,9 +2605,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.22.0

Any objection to me pulling this into a series to clean up similar issues
in arch/x86/kvm/debugfs.c -> kvm_arch_create_vcpu_debugfs(), and to
change kvm_create_vcpu_debugfs() to not return success/failure?  It'd be
nice to fix everything in a single shot.

Re: [PATCH] kvm: remove invalid check for debugfs_create_dir()

[Greg Kroah-Hartman]

On Wed, Jun 12, 2019 at 08:40:21AM -0700, Sean Christopherson wrote:
> On Wed, Jun 12, 2019 at 04:50:33PM +0200, Greg Kroah-Hartman wrote:
> > debugfs_create_dir() can never return NULL, so no need to check for an
> > impossible thing.
> > 
> > It's also not needed to ever check the return value of this function, so
> > just remove the check entirely, and indent the previous line to a sane
> > formatting :)
> > 
> > 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 ca54b09adf5b..4b4ef642d8fa 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2605,9 +2605,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.22.0
> 
> Any objection to me pulling this into a series to clean up similar issues
> in arch/x86/kvm/debugfs.c -> kvm_arch_create_vcpu_debugfs(), and to
> change kvm_create_vcpu_debugfs() to not return success/failure?  It'd be
> nice to fix everything in a single shot.

It was on my todo list to do the cleanup of the x86 kvm stuff.  I
figured this arch-neutral fix was independent, but if you want me to do
it all at once, I'll be glad to do so later this week.

thanks,

greg k-h

Re: [PATCH] kvm: remove invalid check for debugfs_create_dir()

[Sean Christopherson]

On Wed, Jun 12, 2019 at 05:46:22PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 12, 2019 at 08:40:21AM -0700, Sean Christopherson wrote:
> > On Wed, Jun 12, 2019 at 04:50:33PM +0200, Greg Kroah-Hartman wrote:
> > > debugfs_create_dir() can never return NULL, so no need to check for an
> > > impossible thing.
> > > 
> > > It's also not needed to ever check the return value of this function, so
> > > just remove the check entirely, and indent the previous line to a sane
> > > formatting :)
> > > 
> > > 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 ca54b09adf5b..4b4ef642d8fa 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -2605,9 +2605,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.22.0
> > 
> > Any objection to me pulling this into a series to clean up similar issues
> > in arch/x86/kvm/debugfs.c -> kvm_arch_create_vcpu_debugfs(), and to
> > change kvm_create_vcpu_debugfs() to not return success/failure?  It'd be
> > nice to fix everything in a single shot.
> 
> It was on my todo list to do the cleanup of the x86 kvm stuff.  I
> figured this arch-neutral fix was independent, but if you want me to do
> it all at once, I'll be glad to do so later this week.

My preference would be to get it done all at once, having a discrepancy
between kvm_main and x86 leads to a bit of head scratching.

The x86 change would be independent, the piece that's not is converting
kvm_create_vcpu_debugfs() to handle failure internally and not return
errors, which would depend on this patch's removal of "return -ENOMEM".
At a minimum I think that change should be paired with this fix.