On 25/10/19 16:48, Sean Christopherson wrote:
>> It seems to me that kvm_get_kvm() in
>> kvm_arch_init_vm() should be okay as long as it is balanced in
>> kvm_arch_destroy_vm(). So we can apply patch 2 first, and then:
> No, this will effectively leak the VM because you'll end up with a cyclical
> reference to kvm_put_kvm(), i.e. users_count will never hit zero.
>
> void kvm_put_kvm(struct kvm *kvm)
> {
> if (refcount_dec_and_test(&kvm->users_count))
> kvm_destroy_vm(kvm);
> |
> -> kvm_arch_destroy_vm()
> |
> -> kvm_put_kvm()
> }
There's two parts to this:
- if kvm_arch_init_vm() calls kvm_get_kvm(), then kvm_arch_destroy_vm()
won't be called until the corresponding kvm_put_kvm().
- if the error case causes kvm_arch_destroy_vm() to be called early,
however, that'd be okay and would not leak memory, as long as
kvm_arch_destroy_vm() detects the situation and calls kvm_put_kvm() itself.
One case could be where you have some kind of delayed work, where the
callback does kvm_put_kvm. You'd have to cancel the work item and call
kvm_put_kvm in kvm_arch_destroy_vm, and you would go through that path
if kvm_create_vm() fails after kvm_arch_init_vm().
Paolo
