> Date: Fri, 2 Aug 2019 13:29:37 +0300
> From: Paul Irofti <p...@irofti.net>
> 
> On Mon, Jul 01, 2019 at 10:32:51AM +0200, Mark Kettenis wrote:
> > > Date: Thu, 27 Jun 2019 15:08:00 +0300
> > > From: Paul Irofti <p...@irofti.net>
> > > 
> > > Hi,
> > > 
> > > Here is an initial diff, adapted from NetBSD, that synchronizes TSC
> > > clocks across cores.
> > > 
> > > CPU0 is the reference clock and all others are skewed. During CPU
> > > initialization the clocks synchronize by keeping a registry of each CPU
> > > clock skewness and adapting the TSC read routine accordingly.
> > > 
> > > I choose this implementation over what FreeBSD is doing (which is just
> > > copying Linux really), because it is clean and elegant.
> > > 
> > > I would love to hear reports from machines that were broken by this.
> > > Mine, which never exhibited the problem in the first place, run just
> > > fine with the following diff. In fact I am writting this message on one
> > > such machine.
> > > 
> > > Also constructive comments are more than welcomed!
> > > 
> > > Notes:
> > > 
> > > - cpu_counter_serializing() could probably have a better name
> > >   (tsc _read for example)
> > > - the PAUSE instruction is probably not needed
> > > - acpi(4) suspend and resume bits are left out on purpose, but should
> > >   be trivial to add once the current diff settles
> > > 
> > > Paul Irofti
> > 
> > I don't think we want to introduce a <machine/tsc.h> header file.
> > 
> > The code suffers from some NetBSD-isms, so that'll need to be fixed.
> > I pointed some of them out below.
> > 
> > Also, how accurate is your skew detection?  What skew is detected on a
> > machine that (supposedly) has the TSCs in sync?  The result will be
> > that you actually slightly desync the counters on different CPUs.
> > 
> > I think Linux uses the TSC_ADJUST MSR and compares its value across
> > cores.  If the skew is small and the TSC_ADJUST values are the same
> > across cores it skips the TSC adjustments.
> 
> Hi,
> 
> Here is an updated diff with a few bugs eliminated from the previous and
> with most of the concerns I got in private and from Mark fixed.
> 
> I will do the TSC_ADJUST_MSR dance in another iteration if the current
> incarnation turns out to be correct for machines suffering from TSCs not
> in sync.
> 
> The thing I am mostly worried about now is in the following sum
> 
>  uint
>  tsc_get_timecount(struct timecounter *tc)
>  {
>       return rdtsc() + curcpu()->cpu_cc_skew;
>  }
>  
> can one term be executed on one CPU and the other on another? Is there a
> way to protect this from happening other than locking?

Our kernel is non-preemptable so a context switch will only happen if
you sleep.  So that isn't an issue.

> I see NetBSD is checking for a change in the number of context switches 
> of the current process.

Reply via email to