Re: [LTP] Fwd: [PATCH] add calibrate_busy_work_ms()

[Luis C. R. G. <lcl...@uu...>]

On Mon, May 26, 2008 at 01:11:52PM +0530, Chirag Jog wrote:
| * Subrata Modak <su...@li...> [2008-05-26 11:10:45]:
| 
| > Hi,
| > 
| > Please review this patch and provide your comments.
| > 
| > Regards--
| > Subrata
| > 
| > -------- Forwarded Message --------
| > From: Luis Claudio R. Goncalves <lcl...@uu...>
| > To: dvhltc <dv...@li...>, ltp-list
| > <ltp...@li...>
| > Subject: [LTP] [PATCH] add calibrate_busy_work_ms()
| > Date: Sun, 25 May 2008 13:13:43 -0300
| > 
| > [ Please, keep me on Cc: if you have any comments ]
| > 
| > The busy_work_ms() function was not honoring the requested busy interval.
| > 
| > In my test box busy_work_ms(1) would create a 42us busy interval.
| > 
| > To solve this issue I added a function calibrate_busy_work(), called by
| > rt_init(). This funtion runs CALIBRATION_LOOPS and uses the average value
| > as the calibration magic constant.
| > 
| > Two notes:
| > 
| >  * I avoided touching busy_work_us() so that we have a better resolution for
| >    busy_work_ms(). The only user for busy_work_us() as of now is JVMSIM.
| > 
| >  * After adding this fix, I understand the tests are working as supposed to,
| >    with the right amount of busy work. And the fail rate increased.
| 
| 
| 
| Hi, 
|      I added a rt_gettime() before and after busy_work_ms in
|      sched_latency.
| 
|      Without the patch, The result i see is
| 
|      Time spent:     1043 us, Time that should be spent        1 ms
|      Time spent:     1030 us, Time that should be spent        1 ms
	...
| 
|      With the patch applied, I see this:
| 
|      Time spent:       38 us, Time that should be spent        1 ms
|      Time spent:       35 us, Time that should be spent        1 ms
	... 
|      Am i missing something, here?

Hi Chirag,

My experience was exactly the opposite and that lead me to write the patch.
Without that patch, busy_work_ms(1) would create load for 42us on one of my
test boxes and ~37us on the other one.

If you look at the calibrate_busy_work() function, you will notice that it
uses the average busy time from 20 runs of busy_work_ms(1) to calculate the
magic constant. The only way to have busy_work intervals as short as 35us
would be having the magic constant close to 1 - that on the other hand
would mean busy_work_ms(1) kept system busy for almost 1ms.

Other possibilities would be clock skews or problems with VDSO
gettimeofday.

This patch was originally written based on the February tarball of LTP but
I have checked out CVS before sending the patch to the list and the affected
code is essentially the same.

Would that be possible repeating the test and placing the time stats inside
of calibrate_busy_work? This way you could check both original timings
(before the mutiplier takes place) and the magic constant.

Thanks for testing and please keep me posted,
Luis

 
| > Sign-off-by: Luis Claudio R. Goncalves <lcl...@uu...>
| > 
| > diff --git a/testcases/realtime/lib/librttest.c b/testcases/realtime/lib/librttest.c
| > index 6981376..49f101f 100644
| > --- a/testcases/realtime/lib/librttest.c
| > +++ b/testcases/realtime/lib/librttest.c
| > @@ -60,6 +60,8 @@
| >  #include <fcntl.h>
| >  #include <math.h>
| > 
| > +#define CALIBRATION_LOOPS	20
| > +
| >  static LIST_HEAD(_threads);
| >  static atomic_t _thread_count = {-1};
| > 
| > @@ -68,6 +70,8 @@ pthread_mutex_t _buffer_mutex;
| >  int _print_buffer_offset = 0;
| >  int _dbg_lvl = 0;
| >  double pass_criteria;
| > +/* multiplier to calibrate busy_work loop */
| > +static int busy_work_mult = 1;
| > 
| >  static int _use_pi = 1;
| > 
| > @@ -82,6 +86,23 @@ void rt_help(void)
| >  	printf("  -c		Set pass criteria\n");
| >  }
| > 
| > +
| > +static void calibrate_busy_work( void)
| > +{
| > +	int i;
| > +	int delta = 0;
| > +	nsec_t start, end;
| > +
| > +	for(i = 0; i < CALIBRATION_LOOPS; i++) { 
| > +		start = rt_gettime();
| > +		busy_work_ms(1);
| > +		end = rt_gettime();
| > +		delta = delta + (end - start);
| > +	}
| > +	busy_work_mult = (i * NS_PER_MS) / delta;
| > +}
| > +
| > +
| >  int rt_init(const char *options, int (*parse_arg)(int option, char *value), int argc, char *argv[])
| >  {
| >  	int use_buffer = 1;
| > @@ -140,6 +164,8 @@ int rt_init(const char *options, int (*parse_arg)(int option, char *value), int
| >  	if (use_buffer)
| >  		buffer_init();
| > 
| > +	calibrate_busy_work();
| > +
| >  	/*
| >  	 * atexit() order matters here - buffer_print() will be called before
| >  	 * buffer_fini().
| > @@ -428,7 +446,7 @@ nsec_t rt_gettime(void) {
| > 
| >  void *busy_work_ms(int ms)
| >  {
| > -	busy_work_us(ms*US_PER_MS);
| > +	busy_work_us(ms * busy_work_mult);
| >  	return NULL;
| >  }
| > 
| > @@ -438,22 +462,17 @@ void *busy_work_us(int us)
| >  	int scale;
| >  	double pi_scaled;
| >  	double pi_value;
| > -	nsec_t start, now;
| > -	int delta; /* time in us */
| >  	volatile double a=16, b=1.0, c=5.0, d=4, e=1.0, f=239.0;
| > 
| > -	scale = us * ITERS_PER_US;
| > +	/* Calc may take more than 1us on some boxes. Lower this time and
| > +	   guarantee that busy_work_ms is accurate. Tests call busy_work_ms */
| > +	scale = us;
| >  	pi_scaled = 0;
| > -	start = rt_gettime();
| >  	for (i = 0; i < scale; i++) {
| >  		double pi = a*atan(b/c) - d*atan(e/f);
| >  		pi_scaled += pi;
| >  	}
| >  	pi_value = pi_scaled / scale;
| > -	now = rt_gettime();
| > -	delta = (now - start)/NS_PER_US;
| > -	/* uncomment to tune to your machine */
| > -        /* printf("busy_work_us requested: %dus  actual: %dus\n", us, delta); */
| >  	return NULL;
| >  }
| > 
| > 
| 
| -- 
| Cheers,
| Chirag Jog
---end quoted text---

-- 
[ Luis Claudio R. Goncalves                    Bass - Gospel - RT ]
[ Fingerprint: 4FDD B8C4 3C59 34BD 8BE9  2696 7203 D980 A448 C8F8 ]