Re: [Question] remote_tlb_flush statistic is missed from kvm_flush_remote_tlbs() ?

[Marc Zyngier <maz@kernel.org>]

On Fri, 17 Jun 2022 11:33:16 +0100,
Andrew Jones <drjones@redhat.com> wrote:
> 
> On Fri, Jun 17, 2022 at 11:19:28AM +0100, Marc Zyngier wrote:
> > On Fri, 17 Jun 2022 11:02:44 +0100,
> > Gavin Shan <gshan@redhat.com> wrote:
> > > 
> > > Hi Folks,
> > > 
> > > We're reviewing upstream commits and found that it seems that
> > > ++kvm->stat.generic.remote_tlb_flush has been missed from
> > > kvm_flush_remote_tlbs(). If I'm correct, we still need to
> > > increase the statistic in kvm_flush_remote_tlbs()?
> > > 
> > > History about the changes:
> > > 
> > > ce6a7007048b staging: r8188eu: remove {read,write}_macreg
> > >     The changes were NOT there any more.
> > > 419025b3b419 Merge branch kvm-arm64/misc-5.15 into kvmarm-master/next
> > >     The changes were still there
> > > 38f703663d4c KVM: arm64: Count VMID-wide TLB invalidations
> > >     The changes were initially introduced by this commit,
> > >     to increase 'kvm->stat.generic.remote_tlb_flush' in
> > >     kvm_flush_remote_tlbs().
> > 
> > I'm not sure what you are asking. This change is definitely still
> > present in the upstream kernel, and I don't get your point with the
> > staging commit, which is totally unrelated.
> > 
> > $ git describe --contains ce6a7007048b --match=v\*
> > v5.15-rc1~154^2~11
> > $ git describe --contains 419025b3b419 --match=v\*
> > v5.15-rc1~65^2~4^2
> > $ git describe --contains 38f703663d4c --match=v\*
> > v5.15-rc1~65^2~4^2^2~13
> > 
> > As you can see, the commit fixing the statistics was merged after
> > staging one (it appears closer to -rc1, as there is 6 days between the
> > two merge commits from Linus).
> >
> 
> Hi Marc,
> 
> I don't see the change for commit 38f703663d4c as of an upstream pull
> right now
>
> $ git show 47700948a4ab:arch/arm64/kvm/mmu.c | grep -A4 'void kvm_flush_remote_tlbs'
> void kvm_flush_remote_tlbs(struct kvm *kvm)
> {
> 	++kvm->stat.generic.remote_tlb_flush_requests;
> 	kvm_call_hyp(__kvm_tlb_flush_vmid, &kvm->arch.mmu);
> }
> 
> and I do see it got dropped with merge commit e99314a340d2.
> 
> $ git diff 419025b3b419 0d0a19395baa -- arch/arm64/kvm/mmu.c | grep -A5 'void kvm_flush_remote_tlbs'
>  void kvm_flush_remote_tlbs(struct kvm *kvm)
>  {
> +	++kvm->stat.generic.remote_tlb_flush_requests;
>  	kvm_call_hyp(__kvm_tlb_flush_vmid, &kvm->arch.mmu);
> -	++kvm->stat.generic.remote_tlb_flush;
>  }

That's because these two counters should have the exact same value at
all times. We don't use IPIs for TLB invalidation as the HW does the
broadcast for us, so there is no point in distinguishing requests from
actual flushes.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm