On Thu, Mar 26, 2015 at 03:24:10PM -0700, Andy Lutomirski wrote:
> On Thu, Mar 26, 2015 at 3:22 PM, Marcelo Tosatti <mtosatti@xxxxxxxxxx> wrote:
> > On Thu, Mar 26, 2015 at 09:59:24PM +0100, Radim Krčmář wrote:
> >> 2015-03-23 20:21-0300, Marcelo Tosatti:
> >> >
> >> > The following point:
> >> >
> >> > 2. per-CPU pvclock time info is updated if the
> >> > underlying CPU changes.
> >> >
> >> > Is not true anymore since "KVM: x86: update pvclock area conditionally,
> >> > on cpu migration".
> >> >
> >> > Add task migration notification back.
> >> >
> >> > Problem noticed by Andy Lutomirski.
> >> >
> >> > Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx>
> >> > CC: stable@xxxxxxxxxx # 3.11+
> >>
> >> Revert contains a bug that got pointed out in the discussion:
> >>
> >> > diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
> >> > do {
> >> > cpu = __getcpu() & VGETCPU_CPU_MASK;
> >> >
> >> > pvti = get_pvti(cpu);
> >>
> >> We can migrate to 'other cpu' here.
> >>
> >> > + migrate_count = pvti->migrate_count;
> >> > +
> >> > version = __pvclock_read_cycles(&pvti->pvti, &ret, &flags);
> >>
> >> And migrate back to 'cpu' here.
> >
> > Migrating back will increase pvti->migrate_count, right ?
>
> I thought it only increased the count when we migrated away.
Right.
> --Andy
>
> >
> >> rdtsc was executed on different cpu, so pvti and tsc might not be in
> >> sync, but migrate_count hasn't changed.
> >>
> >> > cpu1 = __getcpu() & VGETCPU_CPU_MASK;
> >>
> >> (Reading cpuid here is useless.)
> >>
> >> > } while (unlikely(cpu != cpu1 ||
> >> > (pvti->pvti.version & 1) ||
> >> > - pvti->pvti.version != version));
> >> > + pvti->pvti.version != version ||
> >> > + pvti->migrate_count != migrate_count));
> >>
> >> We can workaround the bug with,
> >>
> >> cpu = __getcpu() & VGETCPU_CPU_MASK;
> >> pvti = get_pvti(cpu);
> >> migrate_count = pvti->migrate_count;
> >> if (cpu != (__getcpu() & VGETCPU_CPU_MASK))
> >> continue;
Looks good, please submit a fix.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html