Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 1 Jun 2011 16:55:49 -0400
From:      Jung-uk Kim <jkim@FreeBSD.org>
To:        Andriy Gapon <avg@freebsd.org>
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: [RFC] Enabling invariant TSC timecounter on SMP
Message-ID:  <201106011655.51233.jkim@FreeBSD.org>
In-Reply-To: <4DE5D0D1.1030903@FreeBSD.org>
References:  <201105241356.45543.jkim@FreeBSD.org> <201105311616.31256.jkim@FreeBSD.org> <4DE5D0D1.1030903@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 01 June 2011 01:40 am, Andriy Gapon wrote:
> on 31/05/2011 23:16 Jung-uk Kim said the following:
> > On Tuesday 31 May 2011 07:18 am, Andriy Gapon wrote:
> >> on 24/05/2011 20:56 Jung-uk Kim said the following:
> >>> I think it's about time to enable invariant TSC timecounter on
> >>> SMP by default.  Please see the attached patch.  It is also
> >>> available from here:
> >>>
> >>> http://people.freebsd.org/~jkim/tsc_smp_test4.diff
> >>>
> >>> avg convinced me enough that it should be an opt-out feature
> >>> going forward. :-)
> >>
> >> Not sure if I really did that.
> >> My position is this:
> >> - if we think that TSC is SMP-safe then it should have the best
> >> priority
> >> - we should do our best to auto-guess if TSC is SMP-safe
> >> unless a user explicitly overrides that property (either via
> >> explicit testing or by making guesses based on CPU model or etc)
> >
> > I am sorry if I misunderstood your intention.  However, I added
> > explicit boot-time TSC sanity check (to do our best to
> > auto-guess) and I think TSC is fairly SMP-safe.  Hence, I thought
> > that it is about time for the change.
>
> In this case - yes.  But I remember that you were thinking about
> either improving or simplifying that check or both.

Yes, it's still a work-in-progress.  However, I thought it is good 
enough for 9.0 inclusion.  BTW, the latest patch is here:

http://people.freebsd.org/~jkim/tsc_smp_test5.diff

FYI, the only meaningful change from the previous version is that it's 
limited to AMD single-socket Bulldozer platforms and Intel Core and 
later platforms.  We may add more quirks if needed, of course.

> >>> Comments?
> >>
> >> Perhaps I missed it, but I don't remember the "lowres" part of
> >> the patch being discussed.
> >
> > No, it wasn't discussed with you.  Do you see any problem with
> > that code?
>
> I don't see any obvious problem, but I also don't understand
> rationale of using smaller max_freq for the ncpus > 1 case.

Consecutive RDTSCs used on a same CPU is always incremental but we 
cannot 100% guarantee that on two cores, even if TSC is derived from 
the same clock.  I am hoping at least latency difference (I believe 
it's about few tens of cycles max) is "eaten up" by lowering 
resolution.  It's not perfect but it's better than serialization 
(Linux) or heuristics (OpenSolaris), just because there are few rare 
conditions to consider.  Thoughts?

> Maybe these two logical changes should be done as two separate
> commits.

Will be done.

Thanks,

Jung-uk Kim



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201106011655.51233.jkim>