On 2015/10/30 22:56, Thomas Gleixner wrote:
Yang,
On Tue, 27 Oct 2015, Yang Yingliang wrote:
Mask the cycle values before subtraction. So we can use this
validation while the clocksource mask is not 64-bits.
Signed-off-by: Yang Yingliang <[email protected]>
Cc: Thomas Gleixner <[email protected]>
---
kernel/time/timekeeping_internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/time/timekeeping_internal.h
b/kernel/time/timekeeping_internal.h
index 4ea005a..984f02e 100644
--- a/kernel/time/timekeeping_internal.h
+++ b/kernel/time/timekeeping_internal.h
@@ -15,7 +15,7 @@ extern void tk_debug_account_sleep_time(struct timespec64 *t);
#ifdef CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE
static inline cycle_t clocksource_delta(cycle_t now, cycle_t last, cycle_t
mask)
{
- cycle_t ret = (now - last) & mask;
+ cycle_t ret = (now & mask) - (last & mask);
I agree the original code is broken for all masks which are !=
CLOCKSOURCE_MASK(64).
But your change does not work for actual wraparounds. You probably
cannot trigger it for the 56bits of the arm architected timer, but
that does not make it more correct.
Assume a CLOCKSOURCE_MASK(32) and that the timer wrapped around since
we last read it.
last = 0xffffffff
now = 0x01
So:
ret = (0x01 & 0xffffffff) - (0xffffffff & 0xffffffff);
--> ret = 0x01 - 0xffffffff;
--> ret = ffffffff00000002;
--> (s64) ret is < 0 !!!
This is wrong as the clocksource legitimately wrapped around since we
accessed it last.
The correct solution to this is:
ret = (now - last) & mask;
negative = ret & ~(mask >> 1);
return negative ? 0 : ret;
Thanks for your advise.
I will resend this patch.
Regards,
Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
