From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Date: Tue, 09 Feb 2021 07:26:02 +0000 Subject: Re: [PATCH 2/2] KVM: PPC: Book3S HV: Optimise TLB flushing when a vcpu moves between threads in a co Message-Id: <20210209072602.GC2841126@thinks.paulus.ozlabs.org> List-Id: References: <20210118122609.1447366-2-npiggin@gmail.com> In-Reply-To: <20210118122609.1447366-2-npiggin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kvm-ppc@vger.kernel.org On Wed, Jan 20, 2021 at 10:26:51AM +1000, Nicholas Piggin wrote: > Excerpts from Aneesh Kumar K.V's message of January 19, 2021 7:45 pm: > > Nicholas Piggin writes: > > > >> As explained in the comment, there is no need to flush TLBs on all > >> threads in a core when a vcpu moves between threads in the same core. > >> > >> Thread migrations can be a significant proportion of vcpu migrations, > >> so this can help reduce the TLB flushing and IPI traffic. > >> > >> Signed-off-by: Nicholas Piggin > >> --- > >> I believe we can do this and have the TLB coherency correct as per > >> the architecture, but would appreciate someone else verifying my > >> thinking. > >> > >> Thanks, > >> Nick > >> > >> arch/powerpc/kvm/book3s_hv.c | 28 ++++++++++++++++++++++++++-- > >> 1 file changed, 26 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c > >> index 752daf43f780..53d0cbfe5933 100644 > >> --- a/arch/powerpc/kvm/book3s_hv.c > >> +++ b/arch/powerpc/kvm/book3s_hv.c > >> @@ -2584,7 +2584,7 @@ static void kvmppc_release_hwthread(int cpu) > >> tpaca->kvm_hstate.kvm_split_mode = NULL; > >> } > >> > >> -static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu) > >> +static void radix_flush_cpu(struct kvm *kvm, int cpu, bool core, struct kvm_vcpu *vcpu) > >> { > > > > Can we rename 'core' to something like 'core_sched' or 'within_core' > > > >> struct kvm_nested_guest *nested = vcpu->arch.nested; > >> cpumask_t *cpu_in_guest; > >> @@ -2599,6 +2599,14 @@ static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu) > >> cpu_in_guest = &kvm->arch.cpu_in_guest; > >> } > >> > > > > and do > > if (core_sched) { > > This function is called to flush guest TLBs on this cpu / all threads on > this cpu core. I don't think it helps to bring any "why" logic into it > because the caller has already dealt with that. I agree with Aneesh that the name "core" doesn't really help the reader to know what the parameter means. Either it needs a comment or a more descriptive name. Paul.