From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: x86: kvm: Revert "remove sched notifier for cross-cpu migrations"
Date: Thu, 26 Mar 2015 19:22:38 -0300
Message-ID: <20150326222238.GB27231@amt.cnet>
References: <20150323232151.GA12772@amt.cnet>
 <20150326205924.GD13271@potion.brq.redhat.com>
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: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:48791 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753250AbbCZWWy (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 26 Mar 2015 18:22:54 -0400
Content-Disposition: inline
In-Reply-To: <20150326205924.GD13271@potion.brq.redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, Mar 26, 2015 at 09:59:24PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 w=
rote:
> 2015-03-23 20:21-0300, Marcelo Tosatti:
> >=20
> > 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 conditiona=
lly,
> > on cpu migration".
> >=20
> > Add task migration notification back.
> >=20
> > Problem noticed by Andy Lutomirski.
> >=20
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > CC: stable@kernel.org # 3.11+
>=20
> Revert contains a bug that got pointed out in the discussion:
>=20
> > diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_=
gettime.c
> >  	do {
> >  		cpu =3D __getcpu() & VGETCPU_CPU_MASK;
> > =20
> >  		pvti =3D get_pvti(cpu);
>=20
> We can migrate to 'other cpu' here.
>=20
> > +		migrate_count =3D pvti->migrate_count;
> > +
> >  		version =3D __pvclock_read_cycles(&pvti->pvti, &ret, &flags);
>=20
> And migrate back to 'cpu' here.

Migrating back will increase pvti->migrate_count, right ?

> rdtsc was executed on different cpu, so pvti and tsc might not be in
> sync, but migrate_count hasn't changed.
>=20
> >  		cpu1 =3D __getcpu() & VGETCPU_CPU_MASK;
>=20
> (Reading cpuid here is useless.)
>=20
> >  	} while (unlikely(cpu !=3D cpu1 ||
> >  			  (pvti->pvti.version & 1) ||
> > -			  pvti->pvti.version !=3D version));
> > +			  pvti->pvti.version !=3D version ||
> > +			  pvti->migrate_count !=3D migrate_count));
>=20
> We can workaround the bug with,
>=20
>   	cpu =3D __getcpu() & VGETCPU_CPU_MASK;
>   	pvti =3D get_pvti(cpu);
>   	migrate_count =3D pvti->migrate_count;
>   	if (cpu !=3D (__getcpu() & VGETCPU_CPU_MASK))
>   		continue;