Messages in this thread Patch in this message | | | From | Gabriel Paubert <> | Date | Tue, 28 Oct 2003 12:55:58 +0100 | Subject | Re: gettimeofday resolution seriously degraded in test9 |
| |
On Mon, Oct 27, 2003 at 05:17:38PM -0800, Stephen Hemminger wrote: > Arghh... the patch was being way more agressive than necessary. > tickadj which limits NTP is always 1 (for HZ=1000) so NTP will change > at most 1 us per clock tick. This meant we only had to stop time > for the last us of the interval.
Hmm, I still don't like it. What does it do to timestamping in interrupts in the kernel, especially when there is a burst of interrupts?
If I read it correctly, the time will be frozen between the time the timer interrupt should have arrived and the time it is processed. So the last micosecond of the interval could extend well into the next interval, or do I miss something (I also suspect that it could make PPSKit behave strangely for this reason)?
> > > diff -Nru a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c > --- a/arch/i386/kernel/time.c Mon Oct 27 17:13:22 2003 > +++ b/arch/i386/kernel/time.c Mon Oct 27 17:13:22 2003 > @@ -110,8 +110,9 @@ > * so make sure not to go into next possible interval. > * Better to lose some accuracy than have time go backwards.. > */ > - if (unlikely(time_adjust < 0) && usec > tickadj) > - usec = tickadj; > + if (unlikely(time_adjust < 0) && > + unlikely(usec > tick_usec - tickadj)) > + usec = tick_usec - tickadj;
So wouldn't it be better to put: usec = max(usec, tick_usec)-tickadj; to freeze the time for only a few microseconds even for kernel clients of gettimeofday() when the timer interrupt comes a bit late?
Now if you are looking at the microsecond level of precision, there is something else in which is worth considering as commented in the appended patch. In short, I consider "do it first because it's faster to be a bogus argument", rather do it in the order which should minimize the jitter of the measurements (outb and inb have enough serialization that no other synchronization should be necessary).
I'm not sure that it will result in easily measurable improvements. But it should not hurt either.
The same could be applied to other timer in the file (HPET) and other files but I don't have any machines on which I can test these, and ensuring that the instruction are executed in the right order is a bit more difficult than with inb and outbs.
Regards, Gabriel
===== arch/i386/kernel/timers/timer_tsc.c 1.29 vs edited ===== --- 1.29/arch/i386/kernel/timers/timer_tsc.c Fri Oct 10 23:01:24 2003 +++ edited/arch/i386/kernel/timers/timer_tsc.c Tue Oct 28 11:04:28 2003 @@ -165,8 +165,10 @@ last_offset = ((unsigned long long)last_tsc_high<<32)|last_tsc_low; /* * It is important that these two operations happen almost at - * the same time. We do the RDTSC stuff first, since it's - * faster. To avoid any inconsistencies, we need interrupts + * the same time. The delay of acquiring the spinlock and + * accessing the PIT may be large, especially if the bus is busy + * with DMA transactions, so the rdtsc is done after to limit + * jitter. To avoid any inconsistencies, we need interrupts * disabled locally. */ @@ -175,15 +177,16 @@ * has the SA_INTERRUPT flag set. -arca */ - /* read Pentium cycle counter */ - - rdtsc(last_tsc_low, last_tsc_high); - spin_lock(&i8253_lock); outb_p(0x00, PIT_MODE); /* latch the count ASAP */ count = inb_p(PIT_CH0); /* read the latched count */ count |= inb(PIT_CH0) << 8; + + /* read Pentium cycle counter */ + + rdtsc(last_tsc_low, last_tsc_high); + spin_unlock(&i8253_lock); if (pit_latch_buggy) { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |