[PATCH] tests: drm_fdinfo: Fix zero tolerance checks

[Janusz Krzysztofik]

Hi Peter,

On Monday, 30 September 2024 12:47:45 GMT+2 Peter Senna Tschudin wrote:
> 
> On 16.09.2024 11:03, Janusz Krzysztofik wrote:
> > When we expect an engine to be busy, we check if its reported busy time
> > falls within a +/-5% tolerance range of measurement time period.
> > However, when we expect the engine to be idle, we compare its reported
> > busy time against zero, still with a +/-5% tolerance range, but now
> > calculated against the zero value, then no tolerance at all.  Obviously,
> > such check fails when the reported busy time is not exactly zero.
> > 
> > Compare engine idle time against measurement time period instead of
> > comparing its busy time against zero when we expect the busy time to be
> > close to zero.  As a debugging aid, display messages with the compared
> > values when requested via --debug option or when a failure occurs.
> > 
> 
> Reviewed-by: Peter Senna Tschudin <peter.senna at linux.intel.com>

Thank you, pushed.

Janusz


> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/7742
> > Suggested-by: Chris Wilson <chris.p.wilson at linux.intel.com>
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> > ---
> >  tests/intel/drm_fdinfo.c | 51 ++++++++++++++++++++--------------------
> >  1 file changed, 25 insertions(+), 26 deletions(-)
> > 
> > diff --git a/tests/intel/drm_fdinfo.c b/tests/intel/drm_fdinfo.c
> > index 43216a64e..45d17aaaa 100644
> > --- a/tests/intel/drm_fdinfo.c
> > +++ b/tests/intel/drm_fdinfo.c
> > @@ -107,12 +107,18 @@ static const char *engine_map[] = {
> >  };
> >  
> >  #define __assert_within_epsilon(x, ref, tol_up, tol_down) \
> > -	igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
> > -		     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
> > -		     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of %f)\n",\
> > -		     #x, #ref, (double)(x), \
> > -		     (tol_up) * 100.0, (tol_down) * 100.0, \
> > -		     (double)(ref))
> > +	do { \
> > +		igt_assert_f((double)(x) <= (1.0 + (tol_up)) * (double)(ref) && \
> > +			     (double)(x) >= (1.0 - (tol_down)) * (double)(ref), \
> > +			     "'%s' != '%s' (%f not within +%.1f%%/-%.1f%% tolerance of %f)\n",\
> > +			     #x, #ref, (double)(x), \
> > +			     (tol_up) * 100.0, (tol_down) * 100.0, \
> > +			     (double)(ref)); \
> > +		igt_debug("%f within +%.1f%%/-%.1f%% tolerance of %f\n",\
> > +			  (double)(x), \
> > +			  (tol_up) * 100.0, (tol_down) * 100.0, \
> > +			  (double)(ref)); \
> > +	} while (0)
> >  
> >  #define assert_within_epsilon(x, ref, tolerance) \
> >  	__assert_within_epsilon(x, ref, tolerance, tolerance)
> > @@ -241,10 +247,8 @@ single(int gem_fd, const intel_ctx_t *ctx,
> >  	else
> >  		end_spin(spin_fd, spin, FLAG_SYNC);
> >  
> > -	assert_within_epsilon(val,
> > -			      (flags & TEST_BUSY) && !(flags & TEST_ISOLATION) ?
> > -			      slept : 0.0f,
> > -			      tolerance);
> > +	assert_within_epsilon((flags & TEST_BUSY) && !(flags & TEST_ISOLATION) ? val : slept - val,
> > +			      slept, tolerance);
> >  
> >  	/* Check for idle after hang. */
> >  	if (flags & FLAG_HANG) {
> > @@ -255,7 +259,7 @@ single(int gem_fd, const intel_ctx_t *ctx,
> >  		slept = measured_usleep(batch_duration_ns / 1000);
> >  		val = read_engine_time(gem_fd, e->class) - val;
> >  
> > -		assert_within_epsilon(val, 0, tolerance);
> > +		assert_within_epsilon(slept - val, slept, tolerance);
> >  	}
> >  
> >  	igt_spin_free(spin_fd, spin);
> > @@ -328,11 +332,8 @@ busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> >  
> >  	log_busy(num_classes, val);
> >  
> > -	for (i = 0; i < num_classes; i++) {
> > -		double target = i == e->class ? slept : 0.0f;
> > -
> > -		assert_within_epsilon(val[i], target, tolerance);
> > -	}
> > +	for (i = 0; i < num_classes; i++)
> > +		assert_within_epsilon(i == e->class ? val[i] : slept - val[i], slept, tolerance);
> >  
> >  	gem_quiescent_gpu(gem_fd);
> >  }
> > @@ -405,9 +406,9 @@ most_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> >  	log_busy(num_classes, val);
> >  
> >  	for (i = 0; i < num_classes; i++) {
> > -		double target = slept * busy_class[i];
> > +		double target = slept * busy_class[i] ?: slept;
> >  
> > -		assert_within_epsilon(val[i], target, tolerance);
> > +		assert_within_epsilon(busy_class[i] ? val[i] : slept - val[i], target, tolerance);
> >  	}
> >  	gem_quiescent_gpu(gem_fd);
> >  }
> > @@ -460,9 +461,9 @@ all_busy_check_all(int gem_fd, const intel_ctx_t *ctx,
> >  	log_busy(num_classes, val);
> >  
> >  	for (i = 0; i < num_classes; i++) {
> > -		double target = slept * busy_class[i];
> > +		double target = slept * busy_class[i] ?: slept;
> >  
> > -		assert_within_epsilon(val[i], target, tolerance);
> > +		assert_within_epsilon(busy_class[i] ? val[i] : slept - val[i], target, tolerance);
> >  	}
> >  	gem_quiescent_gpu(gem_fd);
> >  }
> > @@ -601,10 +602,8 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
> >  			else
> >  				end_spin(i915, spin, FLAG_SYNC);
> >  
> > -			assert_within_epsilon(val,
> > -					      flags & TEST_BUSY ?
> > -					      slept : 0.0f,
> > -					      tolerance);
> > +			assert_within_epsilon(flags & TEST_BUSY ? val : slept - val,
> > +					      slept, tolerance);
> >  
> >  			/* Check for idle after hang. */
> >  			if (flags & FLAG_HANG) {
> > @@ -616,7 +615,7 @@ virtual(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
> >  							1000);
> >  				val = read_engine_time(i915, class) - val;
> >  
> > -				assert_within_epsilon(val, 0, tolerance);
> > +				assert_within_epsilon(slept - val, slept, tolerance);
> >  			}
> >  
> >  			igt_spin_free(i915, spin);
> > @@ -724,7 +723,7 @@ virtual_all(int i915, const intel_ctx_cfg_t *base_cfg, unsigned int flags)
> >  						1000);
> >  			val = read_engine_time(i915, class) - val;
> >  
> > -			assert_within_epsilon(val, 0, tolerance);
> > +			assert_within_epsilon(slept - val, slept, tolerance);
> >  		}
> >  
> >  		igt_spin_free(i915, spin);
>