| Message ID | 1634118172-32699-1-git-send-email-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] KVM: x86: directly call wbinvd for local cpu when emulate wbinvd | expand |
Ping -Li > -----邮件原件----- > 发件人: Li,Rongqing <lirongqing@baidu.com> > 发送时间: 2021年10月13日 17:43 > 收件人: x86@kernel.org; kvm@vger.kernel.org; Li,Rongqing > <lirongqing@baidu.com> > 主题: [PATCH][v2] KVM: x86: directly call wbinvd for local cpu when emulate > wbinvd > > directly call wbinvd for local cpu, instead of calling atomic cpumask_set_cpu to > set local cpu, and then check if local cpu needs to run in on_each_cpu_mask > > on_each_cpu_mask is less efficient than smp_call_function_many, since it will > close preempt again and running call function by checking flag with > SCF_RUN_LOCAL. and here wbinvd can be called directly > > In fact, This change reverts commit 2eec73437487 ("KVM: x86: Avoid issuing > wbinvd twice"), since smp_call_function_many is skiping the local cpu (as > description of c2162e13d6e2f), wbinvd is not issued twice > > and reverts commit c2162e13d6e2f ("KVM: X86: Fix missing local pCPU when > executing wbinvd on all dirty pCPUs") too, which fixed the previous patch, when > revert previous patch, it is not needed. > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > diff v2: rewrite commit log > > arch/x86/kvm/x86.c | 13 ++++++------- > 1 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aabd3a2..28c4c72 > 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6991,15 +6991,14 @@ static int kvm_emulate_wbinvd_noskip(struct > kvm_vcpu *vcpu) > return X86EMUL_CONTINUE; > > if (static_call(kvm_x86_has_wbinvd_exit)()) { > - int cpu = get_cpu(); > - > - cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask); > - on_each_cpu_mask(vcpu->arch.wbinvd_dirty_mask, > + preempt_disable(); > + smp_call_function_many(vcpu->arch.wbinvd_dirty_mask, > wbinvd_ipi, NULL, 1); > - put_cpu(); > + preempt_enable(); > cpumask_clear(vcpu->arch.wbinvd_dirty_mask); > - } else > - wbinvd(); > + } > + > + wbinvd(); > return X86EMUL_CONTINUE; > } > > -- > 1.7.1
> On Oct 21, 2021, at 9:16 PM, Li,Rongqing <lirongqing@baidu.com> wrote: > > Ping > > -Li > >> -----邮件原件----- >> 发件人: Li,Rongqing <lirongqing@baidu.com> >> 发送时间: 2021年10月13日 17:43 >> 收件人: x86@kernel.org; kvm@vger.kernel.org; Li,Rongqing >> <lirongqing@baidu.com> >> 主题: [PATCH][v2] KVM: x86: directly call wbinvd for local cpu when emulate >> wbinvd >> >> directly call wbinvd for local cpu, instead of calling atomic cpumask_set_cpu to >> set local cpu, and then check if local cpu needs to run in on_each_cpu_mask >> >> on_each_cpu_mask is less efficient than smp_call_function_many, since it will >> close preempt again and running call function by checking flag with >> SCF_RUN_LOCAL. and here wbinvd can be called directly >> >> In fact, This change reverts commit 2eec73437487 ("KVM: x86: Avoid issuing >> wbinvd twice"), since smp_call_function_many is skiping the local cpu (as >> description of c2162e13d6e2f), wbinvd is not issued twice >> >> and reverts commit c2162e13d6e2f ("KVM: X86: Fix missing local pCPU when >> executing wbinvd on all dirty pCPUs") too, which fixed the previous patch, when >> revert previous patch, it is not needed. >> >> Signed-off-by: Li RongQing <lirongqing@baidu.com> >> --- >> diff v2: rewrite commit log >> >> arch/x86/kvm/x86.c | 13 ++++++------- >> 1 files changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aabd3a2..28c4c72 >> 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -6991,15 +6991,14 @@ static int kvm_emulate_wbinvd_noskip(struct >> kvm_vcpu *vcpu) >> return X86EMUL_CONTINUE; >> >> if (static_call(kvm_x86_has_wbinvd_exit)()) { >> - int cpu = get_cpu(); >> - >> - cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask); >> - on_each_cpu_mask(vcpu->arch.wbinvd_dirty_mask, >> + preempt_disable(); >> + smp_call_function_many(vcpu->arch.wbinvd_dirty_mask, >> wbinvd_ipi, NULL, 1); >> - put_cpu(); >> + preempt_enable(); >> cpumask_clear(vcpu->arch.wbinvd_dirty_mask); >> - } else >> - wbinvd(); >> + } >> + >> + wbinvd(); >> return X86EMUL_CONTINUE; >> } KVM is none of my business, but on_each_cpu_mask() should be more efficient since it would run wbinvd() concurrently locally and remotely (this is a relatively recent change I made). wbinvd() is an expensive operation, and preempt_enable() is cheap, so there should not be complicated tradeoff here. The proposed change prevents running wbinvd() concurrently so theoretically it should cause a 2x slowdown (for this specific piece of code).
> KVM is none of my business, but on_each_cpu_mask() should be more efficient > since it would run wbinvd() concurrently locally and remotely (this is a relatively > recent change I made). Thanks, I see -Li
diff v2: rewrite commit log arch/x86/kvm/x86.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aabd3a2..28c4c72 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6991,15 +6991,14 @@ static int kvm_emulate_wbinvd_noskip(struct kvm_vcpu *vcpu) return X86EMUL_CONTINUE; if (static_call(kvm_x86_has_wbinvd_exit)()) { - int cpu = get_cpu(); - - cpumask_set_cpu(cpu, vcpu->arch.wbinvd_dirty_mask); - on_each_cpu_mask(vcpu->arch.wbinvd_dirty_mask, + preempt_disable(); + smp_call_function_many(vcpu->arch.wbinvd_dirty_mask, wbinvd_ipi, NULL, 1); - put_cpu(); + preempt_enable(); cpumask_clear(vcpu->arch.wbinvd_dirty_mask); - } else - wbinvd(); + } + + wbinvd(); return X86EMUL_CONTINUE; }
directly call wbinvd for local cpu, instead of calling atomic cpumask_set_cpu to set local cpu, and then check if local cpu needs to run in on_each_cpu_mask on_each_cpu_mask is less efficient than smp_call_function_many, since it will close preempt again and running call function by checking flag with SCF_RUN_LOCAL. and here wbinvd can be called directly In fact, This change reverts commit 2eec73437487 ("KVM: x86: Avoid issuing wbinvd twice"), since smp_call_function_many is skiping the local cpu (as description of c2162e13d6e2f), wbinvd is not issued twice and reverts commit c2162e13d6e2f ("KVM: X86: Fix missing local pCPU when executing wbinvd on all dirty pCPUs") too, which fixed the previous patch, when revert previous patch, it is not needed. Signed-off-by: Li RongQing <lirongqing@baidu.com> ---