Re: [PATCH] KVM: x86/pmu: Avoid ternary operator by directly referring to counters->type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/12/2022 12:46 am, Sean Christopherson wrote:
On Mon, Dec 05, 2022, Like Xu wrote:
From: Like Xu <likexu@xxxxxxxxxxx>

In either case, the counters will point to fixed or gp pmc array, and
taking advantage of the C pointer, it's reasonable to use an almost known
mem load operation directly without disturbing the branch predictor.

The compiler is extremely unlikely to generate a branch for this, e.g. gcc-12 uses
setne and clang-14 shifts "fixed" by 30.  FWIW, clang is also clever enough to
use a cmov to load the address of counters, i.e. the happy path will have no taken
branches for either type of counter.

If so, good news for users of the new tool chain. I assume our Linux project is also
to be commended when it comes to supporting legacy issues even if just a little.


Signed-off-by: Like Xu <likexu@xxxxxxxxxxx>
---
  arch/x86/kvm/vmx/pmu_intel.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
index e5cec07ca8d9..28b0a784f6e9 100644
--- a/arch/x86/kvm/vmx/pmu_intel.c
+++ b/arch/x86/kvm/vmx/pmu_intel.c
@@ -142,7 +142,7 @@ static struct kvm_pmc *intel_rdpmc_ecx_to_pmc(struct kvm_vcpu *vcpu,
  	}
  	if (idx >= num_counters)
  		return NULL;
-	*mask &= pmu->counter_bitmask[fixed ? KVM_PMC_FIXED : KVM_PMC_GP];
+	*mask &= pmu->counter_bitmask[counters->type];

In terms of readability, I have a slight preference for the current code as I
don't have to look at counters->type to understand its possible values.
When someone tries to add a new type of pmc type, the code bugs up. And,
this one will make all usage of pmu->counter_bitmask[] more consistent.

Please reconsider this minor diff if it does no harm.



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux