Re: [PATCH 8/9] KVM, VMX: Add support for guest/host-only profiling

[Gleb Natapov <gleb@redhat.com>]

On Tue, Oct 04, 2011 at 01:10:24PM +0200, Avi Kivity wrote:
> On 10/04/2011 11:56 AM, Gleb Natapov wrote:
> >On Tue, Oct 04, 2011 at 11:28:51AM +0200, Avi Kivity wrote:
> >>  On 10/03/2011 05:36 PM, Gleb Natapov wrote:
> >>  >On Mon, Oct 03, 2011 at 05:00:25PM +0200, Avi Kivity wrote:
> >>  >>   On 10/03/2011 03:49 PM, Gleb Natapov wrote:
> >>  >>   >Support guest/host-only profiling by switch perf msrs on
> >>  >>   >a guest entry if needed.
> >>  >>   >
> >>  >>   >@@ -6052,6 +6056,26 @@ static void vmx_cancel_injection(struct kvm_vcpu *vcpu)
> >>  >>   >    	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD, 0);
> >>  >>   >    }
> >>  >>   >
> >>  >>   >+static void atomic_switch_perf_msrs(struct vcpu_vmx *vmx)
> >>  >>   >+{
> >>  >>   >+#ifdef CONFIG_PERF_EVENTS
> >>  >>
> >>  >>   No need for #ifdef (if you also define perf_guest_get_msrs() when
> >>  >>   !CONFIG_PERF_EVENTS).
> >>  >>
> >>  >Yes, but will compiler be smart enough to remove the code of the
> >>  >function completely? It will have to figure that vmx->perf_msrs_cnt is
> >>  >always 0 somehow.
> >>
> >>  It won't, but do we care?
> >>
> >Dead code, that likely to be inlined, on a hot path.
> 
> I mean, CONFIG_KVM && !CONFIG_PERF_EVENTS is an unlikely
> combination.  If you're using kvm, you usually want PERF_EVENTS.
> 
Who knows. Think about someone building appliance with embedded KVM and
trying to achieve minimal code footprint. It is much easier to add ifdefs
at the development stage then trying to figure out later what can be
ifdeffed. If we will do:
 if (!(cnt = perf_guest_get_msrs_count())
	return;

at the beginning of atomic_switch_perf_msrs() then compiler can
eliminate dead code in case of !CONFIG_PERF_EVENTS since
perf_guest_get_msrs_count() will become 0, but this will add two
function calls on vmentry in CONFIG_PERF_EVENTS case.

--
			Gleb.