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.
--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;
--
Andy Lutomirski
AMA Capital Management, LLC
--
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