From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: x86: kvm: Revert "remove sched notifier for cross-cpu migrations"
Date: Wed, 25 Mar 2015 12:09:38 +0100
Message-ID: <20150325110937.GF21522@potion.brq.redhat.com>
References: <20150323232151.GA12772@amt.cnet>
 <20150324153412.GB21710@potion.brq.redhat.com>
 <20150324225908.GB7067@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm-devel <kvm@vger.kernel.org>, stable@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Andy Lutomirski <luto@amacapital.net>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:55158 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752099AbbCYLJp (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 25 Mar 2015 07:09:45 -0400
Content-Disposition: inline
In-Reply-To: <20150324225908.GB7067@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2015-03-24 19:59-0300, Marcelo Tosatti:
> On Tue, Mar 24, 2015 at 04:34:12PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99=
 wrote:
> > 2015-03-23 20:21-0300, Marcelo Tosatti:
> > > The following point:
> > >=20
> > >     2. per-CPU pvclock time info is updated if the
> > >        underlying CPU changes.
> > >=20
> > > Is not true anymore since "KVM: x86: update pvclock area conditio=
nally,
> > > on cpu migration".
> >=20
> > I think that the revert doesn't fix point 2.:  "KVM: x86: update pv=
clock
> > [...]" changed the host to skip clock update on physical CPU change=
, but
> > guest's task migration notifier isn't tied to it at all.
>=20
> "per-CPU pvclock time info is updated if the underlying CPU changes"
> is the same as
> "always perform clock update on physical CPU change".
>=20
> That was a requirement for the original patch, to drop migration
> notifiers.
>=20
> > (Guest can have all tasks pinned, so the revert changed nothing.)
> >=20
> > > Add task migration notification back.
> > >=20
> > > Problem noticed by Andy Lutomirski.
> >=20
> > What is the problem?
> >=20
> > Thanks.
>=20
> The problem is this:
>=20
> T1) guest thread1 on vcpu1.
> T2) guest thread1 on vcpu2.
> T3) guest thread1 on vcpu1.
>=20
> Inside a pvclock read loop.
>=20
> Since the writes by hypervisor of pvclock area are not ordered,=20
> you cannot rely on version being updated _before_=20
> the rest of pvclock data.
>=20
> (in the case above, "has the physical cpu changed" check, inside the
> guests thread1, obviously fails).

Ah, thanks! so the "KVM: x86: update pvclock area conditionally [...]"
has nothing to do with it -- that really confused me.