Re: [PATCH i-g-t] tests/intel/xe_spin_batch: Check duration of preempter execution

[Francois Dugast <francois.dugast@intel.com>]

On Fri, Jun 21, 2024 at 10:28:29AM -0400, Rodrigo Vivi wrote:
> On Fri, Jun 21, 2024 at 01:53:03PM +0200, Francois Dugast wrote:
> > The preempter is expected to execute without waiting, in a short
> > time. The synchronous call takes roughly 1 ms under normal
> > conditions.
> 
> is this true for any platform?
> 
> I'm asking this because sometimes we add things like this and this
> becomes a 'committed KPI' and later when things start failing on
> a platform we have regressions and escalations on a thing that
> may be normal on that platform.

The batch is minimalistic but I agree with your objection, magic
values for duration and synchronization are not a good idea. Let
us drop this patch.

> 
>  This duration will be higher if the preempter is
> > waiting. For example if the priorities are reversed in this test
> > then this duration will be close to duration_ns (currently set
> > to 100 ms) which was undetected by the test without this change.
> 
> Okay, so the goal of this change is to actually ensure that we
> do not mess up with the test order itself? Any link reference
> to this undetected case mentioned?

The goal of this check was an extra verification of the order of
completion of the 2 tasks. This would have been preparation work
to extend this test later but it makes more sense to create a
separate test, which I do now.

> 
> > 
> > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > ---
> >  tests/intel/xe_spin_batch.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tests/intel/xe_spin_batch.c b/tests/intel/xe_spin_batch.c
> > index 4e95060d3..e57318801 100644
> > --- a/tests/intel/xe_spin_batch.c
> > +++ b/tests/intel/xe_spin_batch.c
> > @@ -253,7 +253,7 @@ static void xe_spin_fixed_duration(int fd, int gt, int class, int flags)
> >  	uint64_t ext = 0;
> >  	size_t bo_size;
> >  	struct xe_spin *spin;
> > -	struct timespec tv;
> > +	struct timespec tv, tv_preempter;
> >  	double elapsed_ms;
> >  	igt_stats_t stats;
> >  	int i;
> > @@ -289,9 +289,11 @@ static void xe_spin_fixed_duration(int fd, int gt, int class, int flags)
> >  		igt_gettime(&tv);
> >  		xe_exec(fd, &exec);
> >  		xe_spin_wait_started(spin);
> > -		if (flags & SPIN_FIX_DURATION_PREEMPT)
> > +		if (flags & SPIN_FIX_DURATION_PREEMPT) {
> > +			igt_gettime(&tv_preempter);
> >  			preempter(fd, hwe);
> > -
> > +			igt_assert(igt_nsec_elapsed(&tv_preempter) * 1e-6 < 10);
> > +		}
> >  		igt_assert(syncobj_wait(fd, &sync.handle, 1, INT64_MAX, 0, NULL));
> >  		igt_stats_push_float(&stats, igt_nsec_elapsed(&tv) * 1e-6);
> >  		syncobj_reset(fd, &sync.handle, 1);
> > -- 
> > 2.43.0
> >