From: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
To: Liang Chen <liangchen.linux@gmail.com>, pbonzini@redhat.com
Cc: rkrcmar@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/2] KVM: x86: directly use kvm_make_request again
Date: Fri, 19 Sep 2014 14:12:25 +0800 [thread overview]
Message-ID: <541BC949.4050602@linux.vnet.ibm.com> (raw)
In-Reply-To: <1411058317-23646-3-git-send-email-liangchen.linux@gmail.com>
On 09/19/2014 12:38 AM, Liang Chen wrote:
> A one-line wrapper around kvm_make_request does not seem
> particularly useful. Replace kvm_mmu_flush_tlb() with
> kvm_make_request() again to free the namespace a bit.
>
> Signed-off-by: Liang Chen <liangchen.linux@gmail.com>
> ---
> arch/x86/include/asm/kvm_host.h | 1 -
> arch/x86/kvm/mmu.c | 16 +++++-----------
> arch/x86/kvm/vmx.c | 2 +-
> arch/x86/kvm/x86.c | 11 ++++++++---
> 4 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 7c492ed..77ade89 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -917,7 +917,6 @@ void kvm_inject_nmi(struct kvm_vcpu *vcpu);
>
> int fx_init(struct kvm_vcpu *vcpu);
>
> -void kvm_mmu_flush_tlb(struct kvm_vcpu *vcpu);
> void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
> const u8 *new, int bytes);
> int kvm_mmu_unprotect_page(struct kvm *kvm, gfn_t gfn);
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index b41fd97..acc2d0c5 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1749,7 +1749,7 @@ static int __kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> return 1;
> }
>
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> return 0;
> }
>
> @@ -1802,7 +1802,7 @@ static void kvm_sync_pages(struct kvm_vcpu *vcpu, gfn_t gfn)
>
> kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
> if (flush)
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> }
>
> struct mmu_page_path {
> @@ -2536,7 +2536,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
> true, host_writable)) {
> if (write_fault)
> *emulate = 1;
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> }
>
> if (unlikely(is_mmio_spte(*sptep) && emulate))
> @@ -3450,12 +3450,6 @@ static void nonpaging_init_context(struct kvm_vcpu *vcpu,
> context->nx = false;
> }
>
> -void kvm_mmu_flush_tlb(struct kvm_vcpu *vcpu)
> -{
> - kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> -}
> -EXPORT_SYMBOL_GPL(kvm_mmu_flush_tlb);
> -
> void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu)
> {
> mmu_free_roots(vcpu);
> @@ -3961,7 +3955,7 @@ static void mmu_pte_write_flush_tlb(struct kvm_vcpu *vcpu, bool zap_page,
> if (remote_flush)
> kvm_flush_remote_tlbs(vcpu->kvm);
> else if (local_flush)
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> }
>
> static u64 mmu_pte_write_fetch_gpte(struct kvm_vcpu *vcpu, gpa_t *gpa,
> @@ -4222,7 +4216,7 @@ EXPORT_SYMBOL_GPL(kvm_mmu_page_fault);
> void kvm_mmu_invlpg(struct kvm_vcpu *vcpu, gva_t gva)
> {
> vcpu->arch.mmu.invlpg(vcpu, gva);
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> ++vcpu->stat.invlpg;
> }
> EXPORT_SYMBOL_GPL(kvm_mmu_invlpg);
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index bfe11cf..bb0a7ab 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6617,7 +6617,7 @@ static int handle_invept(struct kvm_vcpu *vcpu)
> switch (type) {
> case VMX_EPT_EXTENT_GLOBAL:
> kvm_mmu_sync_roots(vcpu);
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> nested_vmx_succeed(vcpu);
> break;
> default:
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9eb5458..fc3df50 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -726,7 +726,7 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> {
> if (cr3 == kvm_read_cr3(vcpu) && !pdptrs_changed(vcpu)) {
> kvm_mmu_sync_roots(vcpu);
> - kvm_mmu_flush_tlb(vcpu);
> + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
> return 0;
> }
>
> @@ -5989,6 +5989,12 @@ static void vcpu_scan_ioapic(struct kvm_vcpu *vcpu)
> kvm_apic_update_tmr(vcpu, tmr);
> }
>
> +static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu)
> +{
> + ++vcpu->stat.tlb_flush;
> + kvm_x86_ops->tlb_flush(vcpu);
> +}
> +
> /*
> * Returns 1 to let __vcpu_run() continue the guest execution loop without
> * exiting to the userspace. Otherwise, the value will be returned to the
> @@ -6018,8 +6024,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> if (kvm_check_request(KVM_REQ_MMU_SYNC, vcpu))
> kvm_mmu_sync_roots(vcpu);
> if (kvm_check_request(KVM_REQ_TLB_FLUSH, vcpu)) {
> - ++vcpu->stat.tlb_flush;
> - kvm_x86_ops->tlb_flush(vcpu);
> + kvm_vcpu_flush_tlb(vcpu);
NACK!
Do not understand why you have to introduce a meaningful name
here - it's used just inner a function, which can not help to
improve a readability of the code at all.
What i suggested is renaming kvm_mmu_flush_tlb() since it's a
API used in multiple files - a good name helps developer to
know what it's doing and definitely easier typing.
next prev parent reply other threads:[~2014-09-19 6:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-18 16:38 [PATCH v3 0/2] KVM: count actual tlb flushes Liang Chen
2014-09-18 16:38 ` [PATCH v3 1/2] KVM: x86: " Liang Chen
2014-09-18 16:38 ` [PATCH v3 2/2] KVM: x86: directly use kvm_make_request again Liang Chen
2014-09-18 18:12 ` Radim Krčmář
2014-09-19 6:12 ` Xiao Guangrong [this message]
2014-09-19 12:25 ` Radim Krčmář
2014-09-19 13:35 ` Xiao Guangrong
2014-09-19 14:00 ` Paolo Bonzini
2014-09-19 14:26 ` Liang Chen
2014-09-19 21:10 ` Radim Krčmář
2014-09-19 14:08 ` Liang Chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=541BC949.4050602@linux.vnet.ibm.com \
--to=xiaoguangrong@linux.vnet.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=liangchen.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).