get_cyclecount(9) deprecation

[Bruce Evans]

On Thu, 17 Mar 2011, Jung-uk Kim wrote:

> Both get_cyclecount(9) and cpu_ticks() do almost exactly the same
> thing now assuming set_cputicker() is called with a correct function
> before get_cyclecount() is used, which is true for x86, at least.
> The only difference is get_cyclecount() may be inlined but I don't
> see much gain from the current uses.
>
> Please review the patch.  Note I didn't really remove get_cyclecount()
> just because some random third-party module may use it as it is a
> documented feature while cpu_ticks is an internal KPI.
>
> What do you think?

I like this idea, but see some minor problems:
- cpu_ticks() API needs to become more public and stable, and guarantee to
   use the ticker with the highest frequency
- it is hard to see when set_cputicker() is called, but on i386 it is
   called very early so there seem to be no problems with the dummy
   timecounter.  It is called from init_TSC() which is called from
   startrtclock() which is called from machdep.c:cpu_startup() just
   after the garbage "Good morning" comment which was originally just
   banal since it preceeded its code (the printf that prints the copyright
   message).  The ordering of this is now highly obfuscated, and in fact
   there is now the following enormous amount of code between the printf
   and its comment:

% enum sysinit_sub_id {
% 	SI_SUB_DUMMY		= 0x0000000,	/* not executed; for linker*/
% 	SI_SUB_DONE		= 0x0000001,	/* processed*/
% 	SI_SUB_TUNABLES		= 0x0700000,	/* establish tunable values */
% 	SI_SUB_COPYRIGHT	= 0x0800001,	/* first use of console*/
   	       ^^^^^^^^^ prints the copyright
% 	SI_SUB_SETTINGS		= 0x0880000,	/* check and recheck settings */
% 	SI_SUB_MTX_POOL_STATIC	= 0x0900000,	/* static mutex pool */
% 	SI_SUB_LOCKMGR		= 0x0980000,	/* lockmgr locks */
% 	SI_SUB_VM		= 0x1000000,	/* virtual memory system init*/
% 	SI_SUB_KMEM		= 0x1800000,	/* kernel memory*/
% 	SI_SUB_KVM_RSRC		= 0x1A00000,	/* kvm operational limits*/
% 	SI_SUB_WITNESS		= 0x1A80000,	/* witness initialization */
% 	SI_SUB_MTX_POOL_DYNAMIC	= 0x1AC0000,	/* dynamic mutex pool */
% 	SI_SUB_LOCK		= 0x1B00000,	/* various locks */
% 	SI_SUB_EVENTHANDLER	= 0x1C00000,	/* eventhandler init */
% 	SI_SUB_VNET_PRELINK	= 0x1E00000,	/* vnet init before modules */
% 	SI_SUB_KLD		= 0x2000000,	/* KLD and module setup */
% 	SI_SUB_CPU		= 0x2100000,	/* CPU resource(s)*/
   	       ^^^ calls cpu_startup()

   but there seem to be no slow operations (like device initialization) in
   there, so cpu_startup() seems to be called early enough.

   The function names for clock initialization are almost equally rotted:
   - the realtime clock was originally the i8254, and startrtclock()
     started it.  Now startrtclock() doesn't touch either the the realtime
     clock (the timecounter) or the i8254.  It only calls init_TSC().  This
     might as well be called directly.  The i8254 is now initialized (only?)
     by i8254, which is called much earlier, from init386(), so that DELAY()
     works when called early from console drivers.  The same care should be
     taken with initializing the TSC for early use, and would be essential
     if the i8254 support in DELAY() were removed.
   - the correct interface for attaching the realtime clock now seems to be
     cpu_initclocks().  This is now used for the TSC (init_TSC_tc()).  But
     the i8254 is now attached as a timecounter in attimer_attach().
- set_cputicker() has various races and implementation bugs:
   - it has no visible locking.  This may be safe when it is called at boot
     time.  It is also called from tsc_levels_changed().  BTW, these calls
     still don't know about P-state invariance.  They always pass the
     parameter saying that the ticker is variable.  This despite the
     invariance variable being checked to lines after the call that passes
     a hard-coded for variance.
   - the call to set_cputicker() after machdep.tsc_freq changes the ticker
     freqency is missing.  This is a feature if the ticker is variable,
     since the dynamic ticker calibration code can be more accurate than
     a fixed setting, and would undo any fixed setting.  But this code is
     buggy...
- set cputicker() has some design bugs.  It assumes that the tick frequency
   is the same across all CPUs, but the TSC is per-CPU.  I have an old SMP
   system with CPUs of different frequency that can demonstrate bugs from
   this.

Bruce