From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [V5] x86/cpuidle: get accurate C0 value with xenpm tool Date: Tue, 19 May 2015 10:01:22 +0100 Message-ID: <555B1802020000780007B7FC@mail.emea.novell.com> References: <1431580999-7886-1-git-send-email-huaitong.han@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1431580999-7886-1-git-send-email-huaitong.han@intel.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Huaitong Han Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org >>> On 14.05.15 at 07:23, wrote: > @@ -574,6 +597,7 @@ static void acpi_processor_idle(void) > t1 = cpuidle_get_tick(); > /* Trace cpu idle entry */ > TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred); > + update_last_cx_stat(power, cx, t1); > /* Invoke C2 */ > acpi_idle_do_entry(cx); > /* Get end time (ticks) */ > @@ -602,7 +626,7 @@ static void acpi_processor_idle(void) > t1 = cpuidle_get_tick(); > /* Trace cpu idle entry */ > TRACE_4D(TRC_PM_IDLE_ENTRY, cx->idx, t1, exp, pred); > - > + update_last_cx_stat(power, cx, t1); > /* Please instead of deleting the blank line here, add another one after the added line and add ones around the addition in the earlier hunk. > @@ -1172,7 +1196,10 @@ int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat) > { > struct acpi_processor_power *power = processor_powers[cpuid]; > uint64_t idle_usage = 0, idle_res = 0; > - uint64_t usage[ACPI_PROCESSOR_MAX_POWER], res[ACPI_PROCESSOR_MAX_POWER]; > + uint64_t last_state_update_tick, current_stime, current_tick; > + uint64_t usage[ACPI_PROCESSOR_MAX_POWER] = { 0 }; > + uint64_t res_ticks[ACPI_PROCESSOR_MAX_POWER] = { 0 }; > + uint64_t res[ACPI_PROCESSOR_MAX_POWER] = { 0 }; Not yet another array on the stack please - I can't see why you can't get away with just res[]. > --- a/xen/arch/x86/cpu/mwait-idle.c > +++ b/xen/arch/x86/cpu/mwait-idle.c > @@ -536,7 +536,6 @@ static void mwait_idle(void) > return; > } > > - power->last_state = cx; > eax = cx->address; > cstate = ((eax >> MWAIT_SUBSTATE_SIZE) & MWAIT_CSTATE_MASK) + 1; > > @@ -554,6 +553,8 @@ static void mwait_idle(void) > > before = cpuidle_get_tick(); > TRACE_4D(TRC_PM_IDLE_ENTRY, cx->type, before, exp, pred); > + /* Now in CX */ > + update_last_cx_stat(power, cx, before); Again - blank line ahead of the addition please. Also the comment is both wrong and (as pointed out before) lacking a stop. Perhaps - just like in the ACPI driver - just omit it (and fix only the other one a few lines down)? Jan