Re: [igt-dev] [PATCH i-g-t] lib/igt_dummyload: Helper to spin till a specified timeout

Chris Wilson
2019-05-31T06:27:01Z

Quoting Ashutosh Dixit (2019-05-31 03:54:05)
> On Wed, 29 May 2019 23:56:43 -0700, Chris Wilson wrote:
> >
> > Quoting Ashutosh Dixit (2019-05-30 06:26:41)
> > > From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
> > >
> > > Even though similar functionality exists in other tests, perhaps there
> > > is value in having a helper function which generates an artificial
> > > load on an engine by spinning on it for a specified time.
> >
> > That seems very odd. As you aren't doing anything else during the spin,
> > what is its purpose for general testing? i.e. where would one use it.
> > With new interfaces, it is always useful to show how you plan to use it
> > so that we can better understand the bigger picture.
>
> This approach has proved useful to us on new hardware in validating simple
> cases such as: (a) spin batch submission and completion timestamps reported
> by the kernel are the same as the time specified in the test (b) spin
> batches submitted on multiple engines are in fact executing in parallel
> (easy to check due to the rather large timeout). Perhaps these are self
> evident conditions which do not need to be checked at all.

The uAPI that you use here should already be checked as part of the
underlying contracts. I would not use this igt_spin_t interface for that,
hence why I suggest you explain the code that is trying to use for it for
that -- because this function is nothing more than a glorified sleep and
cannot be used for anything but that, nor is accurate enough to measure
that sleep.

> The test is really a simpler version of the functionality already present
> in gem_spin_batch.c (though the test itself doesn't "test" for any
> condition). It is for these reasons that I posted the patch in case people
> thought it was useful. If it's not we can drop it. Thanks!

That test only exists because that interface is used by other tests and
so we verify that the igt_spin_t interface works. If you want to check
set-timeout works by all means add a test and not this apparently
useless helper.
-Chris