>>>> The commit 8e92b6605d introduced the TIME_VALID flag for the C1 state
>>>> if this one is a mwait state assuming the interrupt will be enabled
>>>> before reading the end time of the idle state.
...
>>>> I have been digging through the code and I didn't find any place
>>>> where the interrupts are enabled before reading the time.
Linux is correct as it stands, and the patch proposed here is not correct.
Note that on x86, the "STI" instruction enables interrupts:
static inline void native_safe_halt(void)
{
asm volatile("sti; hlt": : :"memory");
}
We get here via acpi_safe_halt(), which is invoked with interrupts
disabled via the cpuidle enter path.
As it needs to return with interrupts disabled, it hacks them off
again, but not before the actual
interrupt is serviced -- which is what throws off the time-stamps in
this path, and why
the CPUIDLE_FLAG_TIME_INVALID exists.
/*
* Callers should disable interrupts before the call and enable
* interrupts after return.
*/
static void acpi_safe_halt(void)
{
if (!tif_need_resched()) {
safe_halt();
local_irq_disable();
}
}
That said...
I think if ladder and menu were more clever, we could delete
CPUIDLE_FLAG_TIME_INVALID...
Today, we use this flag to set our last_residency to the amount of
time until the predicted
next timer expires. There are only two cases:
First, we could have taken and serviced an interrupt -- all before the timer
would have expired.
In this case, we are rounding up last_residency to the max, the
duration till the next timer
would have expired.
Second, we could take the timer, and we could service it for a long
time, and we'd
return a time that is longer than the expected time. So here we are truncating
down to the expected duration till the next timer.
But in case #1, our "invalid" measurement is actually more accurate than what
we are rounding up to. And in case #2, we don't need a flag to detect it --
indeed menu already checks for that case:
/* Make sure our coefficients do not exceed unity */
if (measured_us > data->next_timer_us)
measured_us = data->next_timer_us;
cheers,
Len Brown, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
