Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1304961

Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount

Show all headers | View raw

On Thu, Jan 07, 2016 at 01:13:41PM -0800, Andy Lutomirski wrote:
> On Thu, Jan 7, 2016 at 1:02 PM, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Mon, Jan 04, 2016 at 03:14:28PM -0800, Andy Lutomirski wrote:
> >> If the clock becomes unstable while we're reading it, we need to
> >> bail.  We can do this by simply moving the check into the seqcount
> >> loop.
> >>
> >> Reported-by: Marcelo Tosatti <mtosatti@redhat.com>
> >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> ---
> >>
> >> Marcelo, how's this?
> >>
> >> arch/x86/entry/vdso/vclock_gettime.c | 12 ++++++------
> >>  1 file changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/arch/x86/entry/vdso/vclock_gettime.c b/arch/x86/entry/vdso/vclock_gettime.c
> >> index 8602f06c759f..1a50e09c945b 100644
> >> --- a/arch/x86/entry/vdso/vclock_gettime.c
> >> +++ b/arch/x86/entry/vdso/vclock_gettime.c
> >> @@ -126,23 +126,23 @@ static notrace cycle_t vread_pvclock(int *mode)
> >>        *
> >>        * On Xen, we don't appear to have that guarantee, but Xen still
> >>        * supplies a valid seqlock using the version field.
> >> -
> >> +      *
> >>        * We only do pvclock vdso timing at all if
> >>        * PVCLOCK_TSC_STABLE_BIT is set, and we interpret that bit to
> >>        * mean that all vCPUs have matching pvti and that the TSC is
> >>        * synced, so we can just look at vCPU 0's pvti.
> >>        */
> >>
> >> -     if (unlikely(!(pvti->flags & PVCLOCK_TSC_STABLE_BIT))) {
> >> -             *mode = VCLOCK_NONE;
> >> -             return 0;
> >> -     }
> >> -
> >>       do {
> >>               version = pvti->version;
> >>
> >>               smp_rmb();
> >>
> >> +             if (unlikely(!(pvti->flags & PVCLOCK_TSC_STABLE_BIT))) {
> >> +                     *mode = VCLOCK_NONE;
> >> +                     return 0;
> >> +             }
> >> +
> >>               tsc = rdtsc_ordered();
> >>               pvti_tsc_to_system_mul = pvti->tsc_to_system_mul;
> >>               pvti_tsc_shift = pvti->tsc_shift;
> >> --
> >> 2.4.3
> >
> > Check it before returning the value (once cleared, it can't be set back
> > to 1), similarly to what was in place before.
> >
> >
> 
> I don't understand what you mean.
> 
> In the old code (4.3 and 4.4), the vdso checks STABLE_BIT at the end,
> which is correct as long as STABLE_BIT can never change from 0 to 1.
> 
> In the -tip code, it's clearly wrong.
> 
> In the code in this patch, it should be correct regardless of how
> STABLE_BIT changes as long as the seqcount works.  Given that the
> performance cost of doing that is zero, I'd rather keep it that way.
> If we're really paranoid, we could move it after the rest of the pvti
> reads and add a barrier, but is there really any host on which that
> matters?
> 
> --Andy
> 
> -- 
> Andy Lutomirski
> AMA Capital Management, LLC

Right, its OK due to version check, thanks.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v2 1/4] x86, vdso, pvclock: Simplify and speed up the  vdso pvclock reader Marcelo Tosatti <mtosatti@redhat.com> - 2016-01-04 21:50 +0100
  Re: [PATCH v2 1/4] x86, vdso, pvclock: Simplify and speed up the vdso  pvclock reader Andy Lutomirski <luto@amacapital.net> - 2016-01-04 23:40 +0100
    Re: [PATCH v2 1/4] x86, vdso, pvclock: Simplify and speed up the  vdso pvclock reader Marcelo Tosatti <mtosatti@redhat.com> - 2016-01-05 00:00 +0100
      [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Andy Lutomirski <luto@kernel.org> - 2016-01-05 00:20 +0100
        Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Marcelo Tosatti <mtosatti@redhat.com> - 2016-01-07 22:10 +0100
          Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Andy Lutomirski <luto@amacapital.net> - 2016-01-07 22:20 +0100
            Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Paolo Bonzini <pbonzini@redhat.com> - 2016-01-07 22:50 +0100
            Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Marcelo Tosatti <mtosatti@redhat.com> - 2016-01-08 20:50 +0100
              Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Andy Lutomirski <luto@amacapital.net> - 2016-01-12 20:50 +0100
                Re: [PATCH] x86/vdso/pvclock: Protect STABLE check with the seqcount Ingo Molnar <mingo@kernel.org> - 2016-01-13 11:50 +0100
        [tip:x86/urgent] x86/vdso/pvclock:   Protect STABLE check with the seqcount tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2016-01-14 10:10 +0100

csiph-web