On Thu, Feb 01, 2007 at 12:36:05PM +0100, Andi Kleen wrote:
> On Thursday 01 February 2007 11:00, [EMAIL PROTECTED] wrote:
> 
> > +           case VXTIME_TSC:
> > +                   rdtscll(tsc);
> 
> Where is the CPU synchronization? 
> 
> > +   cpu = smp_processor_id();
> > +   rdtscll(t);
> 
> Also no synchronization. It's slower, but needed.

Hmm, I wasn't sure. Why is it needed? How outdated can the
result of RDTSC / RDTSCP be?

If I do:
        rdtscll(a)
        ...
        rdtscll(b)
is it guaranteed that (b > a) ?

>
> >  unsigned long long sched_clock(void)
> >  {
> > -   unsigned long a = 0;
> > -
> > -   rdtscll(a);
> > -   return cycles_2_ns(a);
> > +   return monotonic_clock();
> >  }
> 
> This is overkill because sched_clock() doesn't need a globally monotonic
> clock, per CPU monotonic is enough. The old version was fine.

OK, thanks for spotting this. I'll change it to use __guess_mt().
(more or less equal to cycles_2_ns(), no need to maintain yet another
tsc->ns ratio just for cycles_2_ns().


> > +static __always_inline void do_vgettimeofday(struct timeval * tv, u64 tsc, 
> > int cpu)
> > +{
> > +   unsigned int sec;
> > +   s64 nsec;
> >  
> > -   do {
> > -           sequence = read_seqbegin(&__xtime_lock);
> > -           
> > -           sec = __xtime.tv_sec;
> > -           usec = __xtime.tv_nsec / 1000;
> > -
> > -                   usec += ((readl((void __iomem *)
> > -                              fix_to_virt(VSYSCALL_HPET) + 0xf0) -
> > -                             __vxtime.last) * __vxtime.quot) >> 32;
> > -   } while (read_seqretry(&__xtime_lock, sequence));
> > +   sec = __xtime.tv_sec;
> > +   nsec = __xtime.tv_nsec;
> > +   nsec += max(__do_gettimeoffset(tsc, cpu), __vxtime.drift);
> >  
> > -   tv->tv_sec = sec + usec / 1000000;
> > -   tv->tv_usec = usec % 1000000;
> > +   sec += nsec / NSEC_PER_SEC;
> > +   nsec %= NSEC_PER_SEC;
> 
> Using while() here is probably faster (done in vdso patchkit where
> gtod got mysteriously faster). Modulo and divisions are slow, even 
> for constants when they are large.

OK, will do that

> 
> >  }
> >  
> >  /* RED-PEN may want to readd seq locking, but then the variable should be 
> > write-once. */
> > @@ -107,10 +118,39 @@ static __always_inline long time_syscall
> >  
> >  int __vsyscall(0) vgettimeofday(struct timeval * tv, struct timezone * tz)
> >  {
> > -   if (!__sysctl_vsyscall)
> > +   int cpu = 0;
> > +   u64 tsc;
> > +   unsigned long seq;
> > +   int do_syscall = !__sysctl_vsyscall;
> > +
> > +   if (tv && !do_syscall)
> > +           switch (__vxtime.mode) {
> > +                   case VXTIME_TSC:
> > +                   case VXTIME_TSCP:
> > +                           do {
> > +                                   seq = read_seqbegin(&__xtime_lock);
> > +
> > +                                   if (__vxtime.mode == VXTIME_TSC)
> > +                                           rdtscll(tsc);
> > +                                   else {
> > +                                           rdtscpll(tsc, cpu);
> > +                                           cpu &= 0xfff;
> > +                                   }
> > +
> > +                                   if 
> > (unlikely(__vxtime.cpu[cpu].tsc_invalid))
> > +                                           do_syscall = 1;
> > +                                   else
> > +                                           do_vgettimeofday(tv, tsc, cpu);
> > +
> > +                           } while (read_seqretry(&__xtime_lock, seq));
> > +                           break;
> > +                   default:
> > +                           do_syscall = 1;
> 
> Why do you not set __sysctl_vsyscall correctly for the mode at initialization?

Because of the __vxtime.cpu[cpu].tsc_invalid flag. We may be
using the vsyscall, but when we get the cpufreq PRE- notification, we
know that TSC cannot be trusted from that point on, until the
frequency stabilises. We set the flag and until TSC becomes
reliable again, vsyscall w/ HW Master Timer read will be used.

So this is something that changes in runtime, and cannot be set
permanently on initialization...


-- 
Jiri Bohac <[EMAIL PROTECTED]>
SUSE Labs, SUSE CZ

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to