| Message ID | 20190612145033.GA18084@kroah.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: remove invalid check for debugfs_create_dir() | expand |
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.
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
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.
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) {
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(-)