| Message ID | 20231117122633.47028-1-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: fix kvm_has_noapic_vcpu updates when fail to create vcpu | expand |
On Fri, 2023-11-17 at 20:26 +0800, Li RongQing wrote: > Static key kvm_has_noapic_vcpu should be reduced when fail > to create vcpu, this patch fixes it > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > arch/x86/kvm/x86.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 41cce50..2a22e66 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > kfree(vcpu->arch.mci_ctl2_banks); > free_page((unsigned long)vcpu->arch.pio_data); > fail_free_lapic: > - kvm_free_lapic(vcpu); > + if (!lapic_in_kernel(vcpu)) > + static_branch_dec(&kvm_has_noapic_vcpu); > + else > + kvm_free_lapic(vcpu); > fail_mmu_destroy: > kvm_mmu_destroy(vcpu); > return r; Makes sense. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote: > Static key kvm_has_noapic_vcpu should be reduced when fail > to create vcpu, this patch fixes it > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > arch/x86/kvm/x86.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 41cce50..2a22e66 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) > kfree(vcpu->arch.mci_ctl2_banks); > free_page((unsigned long)vcpu->arch.pio_data); > fail_free_lapic: > - kvm_free_lapic(vcpu); > + if (!lapic_in_kernel(vcpu)) > + static_branch_dec(&kvm_has_noapic_vcpu); > + else > + kvm_free_lapic(vcpu); > fail_mmu_destroy: > kvm_mmu_destroy(vcpu); > return r; It is good to me. But is it better also take the chance to tidy up kvm_arch_vcpu_destroy(): kvm_free_lapic(vcpu); idx = srcu_read_lock(&vcpu->kvm->srcu); kvm_mmu_destroy(vcpu); srcu_read_unlock(&vcpu->kvm->srcu, idx); free_page((unsigned long)vcpu->arch.pio_data); kvfree(vcpu->arch.cpuid_entries); if (!lapic_in_kernel(vcpu)) static_branch_dec(&kvm_has_noapic_vcpu); Thanks, Yilun > -- > 2.9.4 > >
> -----Original Message----- > From: Xu Yilun <yilun.xu@linux.intel.com> > Sent: Tuesday, November 21, 2023 11:14 PM > To: Li,Rongqing <lirongqing@baidu.com> > Cc: x86@kernel.org; kvm@vger.kernel.org > Subject: Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to > create vcpu > > On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote: > > Static key kvm_has_noapic_vcpu should be reduced when fail to create > > vcpu, this patch fixes it > > > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > --- > > arch/x86/kvm/x86.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > > 41cce50..2a22e66 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu > *vcpu) > > kfree(vcpu->arch.mci_ctl2_banks); > > free_page((unsigned long)vcpu->arch.pio_data); > > fail_free_lapic: > > - kvm_free_lapic(vcpu); > > + if (!lapic_in_kernel(vcpu)) > > + static_branch_dec(&kvm_has_noapic_vcpu); > > + else > > + kvm_free_lapic(vcpu); > > fail_mmu_destroy: > > kvm_mmu_destroy(vcpu); > > return r; > > It is good to me. But is it better also take the chance to tidy up > kvm_arch_vcpu_destroy(): > > kvm_free_lapic(vcpu); > idx = srcu_read_lock(&vcpu->kvm->srcu); > kvm_mmu_destroy(vcpu); > srcu_read_unlock(&vcpu->kvm->srcu, idx); > free_page((unsigned long)vcpu->arch.pio_data); > kvfree(vcpu->arch.cpuid_entries); > if (!lapic_in_kernel(vcpu)) > static_branch_dec(&kvm_has_noapic_vcpu); > Do you means that calling kvm_free_lapic when lapic_in_kernel is true? diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2c92407..9d176c7 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -12122,14 +12122,17 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) kvm_pmu_destroy(vcpu); kfree(vcpu->arch.mce_banks); kfree(vcpu->arch.mci_ctl2_banks); - kvm_free_lapic(vcpu); + + if (lapic_in_kernel(vcpu)) + kvm_free_lapic(vcpu); + else + static_branch_dec(&kvm_has_noapic_vcpu); + idx = srcu_read_lock(&vcpu->kvm->srcu); kvm_mmu_destroy(vcpu); srcu_read_unlock(&vcpu->kvm->srcu, idx); free_page((unsigned long)vcpu->arch.pio_data); kvfree(vcpu->arch.cpuid_entries); - if (!lapic_in_kernel(vcpu)) - static_branch_dec(&kvm_has_noapic_vcpu); } void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
On Wed, Nov 22, 2023 at 06:15:44AM +0000, Li,Rongqing wrote: > > > > -----Original Message----- > > From: Xu Yilun <yilun.xu@linux.intel.com> > > Sent: Tuesday, November 21, 2023 11:14 PM > > To: Li,Rongqing <lirongqing@baidu.com> > > Cc: x86@kernel.org; kvm@vger.kernel.org > > Subject: Re: [PATCH] KVM: x86: fix kvm_has_noapic_vcpu updates when fail to > > create vcpu > > > > On Fri, Nov 17, 2023 at 08:26:33PM +0800, Li RongQing wrote: > > > Static key kvm_has_noapic_vcpu should be reduced when fail to create > > > vcpu, this patch fixes it > > > > > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > > --- > > > arch/x86/kvm/x86.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index > > > 41cce50..2a22e66 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu > > *vcpu) > > > kfree(vcpu->arch.mci_ctl2_banks); > > > free_page((unsigned long)vcpu->arch.pio_data); > > > fail_free_lapic: > > > - kvm_free_lapic(vcpu); > > > + if (!lapic_in_kernel(vcpu)) > > > + static_branch_dec(&kvm_has_noapic_vcpu); > > > + else > > > + kvm_free_lapic(vcpu); > > > fail_mmu_destroy: > > > kvm_mmu_destroy(vcpu); > > > return r; > > > > It is good to me. But is it better also take the chance to tidy up > > kvm_arch_vcpu_destroy(): > > > > kvm_free_lapic(vcpu); > > idx = srcu_read_lock(&vcpu->kvm->srcu); > > kvm_mmu_destroy(vcpu); > > srcu_read_unlock(&vcpu->kvm->srcu, idx); > > free_page((unsigned long)vcpu->arch.pio_data); > > kvfree(vcpu->arch.cpuid_entries); > > if (!lapic_in_kernel(vcpu)) > > static_branch_dec(&kvm_has_noapic_vcpu); > > > > Do you means that calling kvm_free_lapic when lapic_in_kernel is true? Yes. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2c92407..9d176c7 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -12122,14 +12122,17 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) > kvm_pmu_destroy(vcpu); > kfree(vcpu->arch.mce_banks); > kfree(vcpu->arch.mci_ctl2_banks); > - kvm_free_lapic(vcpu); > + > + if (lapic_in_kernel(vcpu)) > + kvm_free_lapic(vcpu); > + else > + static_branch_dec(&kvm_has_noapic_vcpu); Better keep the same style as in kvm_arch_vcpu_create(): if (!lapic_in_kernel(vcpu)) static_branch_dec(&kvm_has_noapic_vcpu); else kvm_free_lapic(vcpu); Thanks, Yilun > + > idx = srcu_read_lock(&vcpu->kvm->srcu); > kvm_mmu_destroy(vcpu); > srcu_read_unlock(&vcpu->kvm->srcu, idx); > free_page((unsigned long)vcpu->arch.pio_data); > kvfree(vcpu->arch.cpuid_entries); > - if (!lapic_in_kernel(vcpu)) > - static_branch_dec(&kvm_has_noapic_vcpu); > } > > void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > > >
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 41cce50..2a22e66 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11957,7 +11957,10 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) kfree(vcpu->arch.mci_ctl2_banks); free_page((unsigned long)vcpu->arch.pio_data); fail_free_lapic: - kvm_free_lapic(vcpu); + if (!lapic_in_kernel(vcpu)) + static_branch_dec(&kvm_has_noapic_vcpu); + else + kvm_free_lapic(vcpu); fail_mmu_destroy: kvm_mmu_destroy(vcpu); return r;
Static key kvm_has_noapic_vcpu should be reduced when fail to create vcpu, this patch fixes it Signed-off-by: Li RongQing <lirongqing@baidu.com> --- arch/x86/kvm/x86.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)