From mboxrd@z Thu Jan  1 00:00:00 1970
From: Glauber Costa <glommer@redhat.com>
Subject: Re: [PATCH] remove static declaration from wall clock version
Date: Thu, 26 Feb 2009 22:22:22 -0300
Message-ID: <20090227012222.GA5979@poweredge.glommer>
References: <1235677340-3139-1-git-send-email-glommer@redhat.com> <200902262050.27555.arnd@arndb.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, avi@redhat.com
To: Arnd Bergmann <arnd@arndb.de>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:41285 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752804AbZB0BSy (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 26 Feb 2009 20:18:54 -0500
Content-Disposition: inline
In-Reply-To: <200902262050.27555.arnd@arndb.de>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, Feb 26, 2009 at 08:50:26PM +0100, Arnd Bergmann wrote:
> On Thursday 26 February 2009, Glauber Costa wrote:
> > @@ -548,15 +548,13 @@ static int do_set_msr(struct kvm_vcpu *vcpu, =
unsigned index, u64 *data)
> > =A0
> > =A0static void kvm_write_wall_clock(struct kvm *kvm, gpa_t wall_clo=
ck)
> > =A0{
> > -=A0=A0=A0=A0=A0=A0=A0static int version;
> > +=A0=A0=A0=A0=A0=A0=A0int version =3D 1;
> > =A0=A0=A0=A0=A0=A0=A0=A0struct pvclock_wall_clock wc;
> > =A0=A0=A0=A0=A0=A0=A0=A0struct timespec now, sys, boot;
> > =A0
> > =A0=A0=A0=A0=A0=A0=A0=A0if (!wall_clock)
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return;
> > =A0
> > -=A0=A0=A0=A0=A0=A0=A0version++;
> > -
> > =A0=A0=A0=A0=A0=A0=A0=A0kvm_write_guest(kvm, wall_clock, &version, =
sizeof(version));
> > =A0
> > =A0=A0=A0=A0=A0=A0=A0=A0/*
>=20
> Doesn't this mean that kvm_write_guest now writes an uninitialized va=
lue
> to the guest?
No. If you look closely, it's now initialized to 1.

>=20
> I think what you need here is a 'static atomic_t version;' so you can
> do an atomic_inc instead of the ++.

I don't see the need for atomicity. This is just called once, at boot t=
ime.
The only thing we're protecting here is one guest from another. The sta=
ck
will do fine for this.