Subject: Re: separate statclock of footbridge (Re: CVS commit: syssrc/sys/arch/arm/footbridge)
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Chris Gilbert <chris@dokein.co.uk>
List: port-arm
Date: 10/27/2002 14:57:28
On Sun, 27 Oct 2002 23:03:42 +0900 (JST)
Izumi Tsutsui <tsutsui@ceres.dti.ne.jp> wrote:

> In article <20021005122256.17949B42C@cvs.netbsd.org>
> chris@netbsd.org wrote:
> 
> > Module Name:	syssrc
> > Committed By:	chris
> > Date:		Sat Oct  5 12:22:55 UTC 2002
> > 
> > Modified Files:
> > 	syssrc/sys/arch/arm/footbridge: footbridge_clock.c
> > 
> > Log Message:
> > Add random jitter to stat clock, the random jitter is +- 511 usec's,
> > so we should average the nominal clock rate.
> 
> In this change, it seems statprev is not initialized properly
> so it would cause random overruns. Is this patch Ok?
> 
> ---
> Index: footbridge_clock.c
> ===================================================================
> RCS file: /cvsroot/syssrc/sys/arch/arm/footbridge/footbridge_clock.c,v
> retrieving revision 1.13
> diff -u -r1.13 footbridge_clock.c
> --- footbridge_clock.c	2002/10/10 23:19:13	1.13
> +++ footbridge_clock.c	2002/10/27 13:47:08
> @@ -244,7 +244,8 @@
>  	int statvarticks;
>  
>  	/* statint == num in counter to drop by desired hz */
> -	statint = clock_sc->sc_statclock_count =
> load_timer(TIMER_2_BASE, hz);+	statint = statprev =
> clock_sc->sc_statclock_count =+	    load_timer(TIMER_2_BASE, hz);
>  	
>  	/* Get the total ticks a second */
>  	countpersecond = statint * hz;
> 
> ---

Looks fine, my mistake.  Note that you'll still get stat clock overruns
as the current interrupt handling may not deal with them in a timely
fashion (do a build of something if it causes enough disk/network io
then you may find that you get overruns)

> > stathz now runs at hz (the hard clock hz), without getting high
> > amounts of time in interrupt handling.
> 
> BTW, it seems this code is taken from hp300/clock.c,
> but why does `adding random jitter' solve the problem?
> If the problem is caused by conflicts of clockhandler() and
> statclockhandler(), isn't it enough to add some delay (~5ms)
> before initializing TIMER_2 after TIMER_1 is started?

it's solving 2 things:
1 being you don't always take a statclock interrupt at the same time as
the hardclock (or near enough)  But you may just end up always taking a
sample at the same point without the randomness.
2 It also helps track where we really are using time otherwise you could
find that you'll not see a process that's running as it gives up the cpu
too often.  Adding randomness is normal practice (see
sparc/sparc/clock.c) and desired.

Chris