diff mbox series

[v2,4/4] KVM: PPC: Decrement module refcount if init_vm fails

Message ID 20220124220803.1011673-5-farosas@linux.ibm.com
State New
Headers show
Series KVM: PPC: KVM module exit fixes | expand

Commit Message

Fabiano Rosas Jan. 24, 2022, 10:08 p.m. UTC
We increment the reference count for KVM-HV/PR before the call to
kvmppc_core_init_vm. If that function fails we need to decrement the
refcount.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
Caught this while testing Nick's LPID patches by looking at
/sys/module/kvm_hv/refcnt
---
 arch/powerpc/kvm/powerpc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Nicholas Piggin Jan. 25, 2022, 3:46 a.m. UTC | #1
Excerpts from Fabiano Rosas's message of January 25, 2022 8:08 am:
> We increment the reference count for KVM-HV/PR before the call to
> kvmppc_core_init_vm. If that function fails we need to decrement the
> refcount.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> Caught this while testing Nick's LPID patches by looking at
> /sys/module/kvm_hv/refcnt

Nice catch. Is this the only change in the series?

You can just use kvm_ops->owner like try_module_get() does I think? Also
try_module_get works on a NULL module same as module_put by the looks,
so you could adjust that in this patch to remove the NULL check so it
is consistent with the put.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Thanks,
Nick


> ---
>  arch/powerpc/kvm/powerpc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 2ad0ccd202d5..4285d0eac900 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -431,6 +431,8 @@ int kvm_arch_check_processor_compat(void *opaque)
>  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  {
>  	struct kvmppc_ops *kvm_ops = NULL;
> +	int r;
> +
>  	/*
>  	 * if we have both HV and PR enabled, default is HV
>  	 */
> @@ -456,7 +458,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
>  		return -ENOENT;
>  
>  	kvm->arch.kvm_ops = kvm_ops;
> -	return kvmppc_core_init_vm(kvm);
> +	r = kvmppc_core_init_vm(kvm);
> +	if (r)
> +		module_put(kvm->arch.kvm_ops->owner);
> +	return r;
>  err_out:
>  	return -EINVAL;
>  }
> -- 
> 2.34.1
> 
>
Fabiano Rosas Jan. 25, 2022, 3:08 p.m. UTC | #2
Nicholas Piggin <npiggin@gmail.com> writes:

> Excerpts from Fabiano Rosas's message of January 25, 2022 8:08 am:
>> We increment the reference count for KVM-HV/PR before the call to
>> kvmppc_core_init_vm. If that function fails we need to decrement the
>> refcount.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>> Caught this while testing Nick's LPID patches by looking at
>> /sys/module/kvm_hv/refcnt
>
> Nice catch. Is this the only change in the series?

Yes.

> You can just use kvm_ops->owner like try_module_get() does I think? Also
> try_module_get works on a NULL module same as module_put by the looks,
> so you could adjust that in this patch to remove the NULL check so it
> is consistent with the put.

Sure, I'll send a v3.
diff mbox series

Patch

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 2ad0ccd202d5..4285d0eac900 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -431,6 +431,8 @@  int kvm_arch_check_processor_compat(void *opaque)
 int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 {
 	struct kvmppc_ops *kvm_ops = NULL;
+	int r;
+
 	/*
 	 * if we have both HV and PR enabled, default is HV
 	 */
@@ -456,7 +458,10 @@  int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 		return -ENOENT;
 
 	kvm->arch.kvm_ops = kvm_ops;
-	return kvmppc_core_init_vm(kvm);
+	r = kvmppc_core_init_vm(kvm);
+	if (r)
+		module_put(kvm->arch.kvm_ops->owner);
+	return r;
 err_out:
 	return -EINVAL;
 }