Ben Hutchings <b...@decadent.org.uk> writes:
> On Mon, 2010-07-12 at 10:22 +0200, Bjørn Mork wrote:
>
>> But I must admit that I was a bit surprised when 2.6.32-17 was uploaded
>> without a fix for this problem. It makes me wonder if the severity
>> shouldn't have been higher after all.... Just to be clear: This bug
>> does break all KVM based systems, making them completely unbootable, and
>> requiring console access to fix. I'm pretty sure there are plenty of
>> KVM based hosting solutions around where this will be a severe problem.
>
> I haven't seen a fix for it yet.
Ah, sorry, I thought the fix was clear from the bug report:
Peter Palfrader has bisected it down to commit 1345126c
"x86, paravirt: Add a global synchronization point for pvclock"
which sounds very reasonable, IMHO. See
http://lkml.indiana.edu/hypermail/linux/kernel/1007.0/02385.html
To be absolutely sure this is correct, I just built a kernel based on
your 2.6.32-17 with commit 1345126c reverted, and that does indeed boot
under KVM. The revert patch is attached. Please try it.
I do hope this goes into next 2.6.32-stable as well, although the
discussion following Peter Palfrader's bisect seems a bit discouraging.
"that patch shouldn't affect anything outside..." Right. Well, it
does. So please revert it in stable.
Thanks,
Bjørn
>From 5ffe322b5f3a76969525bf2cff3f029297179f27 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bj...@mork.no>
Date: Mon, 12 Jul 2010 13:23:57 +0200
Subject: [PATCH] Revert "x86, paravirt: Add a global synchronization point for pvclock"
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 1345126c761f0360dc108973bf73281d51945bc1.
Signed-off-by: Bjørn Mork <bj...@mork.no>
---
arch/x86/kernel/pvclock.c | 24 ------------------------
1 files changed, 0 insertions(+), 24 deletions(-)
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index dfdfe46..03801f2 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -109,14 +109,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src)
return pv_tsc_khz;
}
-static atomic64_t last_value = ATOMIC64_INIT(0);
-
cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
{
struct pvclock_shadow_time shadow;
unsigned version;
cycle_t ret, offset;
- u64 last;
do {
version = pvclock_get_time_values(&shadow, src);
@@ -126,27 +123,6 @@ cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
barrier();
} while (version != src->version);
- /*
- * Assumption here is that last_value, a global accumulator, always goes
- * forward. If we are less than that, we should not be much smaller.
- * We assume there is an error marging we're inside, and then the correction
- * does not sacrifice accuracy.
- *
- * For reads: global may have changed between test and return,
- * but this means someone else updated poked the clock at a later time.
- * We just need to make sure we are not seeing a backwards event.
- *
- * For updates: last_value = ret is not enough, since two vcpus could be
- * updating at the same time, and one of them could be slightly behind,
- * making the assumption that last_value always go forward fail to hold.
- */
- last = atomic64_read(&last_value);
- do {
- if (ret < last)
- return last;
- last = atomic64_cmpxchg(&last_value, last, ret);
- } while (unlikely(last != ret));
-
return ret;
}
--
1.7.1
