[igt-dev] [PATCH i-g-t 1/2] tests/gem_sync: Use timeouts definitions instead hardcoded value

[Zbigniew Kempczyński]

On Fri, Oct 29, 2021 at 02:32:48PM -0700, Dixit, Ashutosh wrote:
> On Thu, 28 Oct 2021 11:23:21 -0700, Zbigniew Kempczyński wrote:
> >
> > Make developer life easier and use definitions which are useful
> > to shorten time during work on some subtest.
> 
> Hmm, I don't know about this one. If we want to increase 2 to 5 say for
> some tests or reduce 20 to 10 for some others, previously we could just
> change the numbers and here we need to go and #define some more
> short-short, short-medium etc. Can we leave what we have as is? Unless you
> have a strong preference. Thanks.

No, I have no strong preference about this. I can leave it as it is -
I've added these defines because testing during rewrite was very time
consuming and I wanted to shorten it. And increase code readibilty a
bit on the first look.

I'm going to remove this in next series.

--
Zbigniew

> 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Ashutosh Dixit <ashutosh.dixit at intel.com>
> > ---
> >  tests/i915/gem_sync.c | 68 +++++++++++++++++++++++--------------------
> >  1 file changed, 37 insertions(+), 31 deletions(-)
> >
> > diff --git a/tests/i915/gem_sync.c b/tests/i915/gem_sync.c
> > index 0093ca25f..3b9f9889c 100644
> > --- a/tests/i915/gem_sync.c
> > +++ b/tests/i915/gem_sync.c
> > @@ -1150,6 +1150,11 @@ preempt(int fd, const intel_ctx_t *ctx, unsigned ring,
> >	intel_ctx_destroy(fd, tmp_ctx[0]);
> >  }
> >
> > +/* small / medium / large */
> > +#define STIMEOUT 1
> > +#define MTIMEOUT 2
> > +#define LTIMEOUT 20
> > +
> >  igt_main
> >  {
> >	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> > @@ -1160,31 +1165,31 @@ igt_main
> >		int num_children;
> >		int timeout;
> >	} all[] = {
> > -		{ "basic-each", sync_ring, 1, 2 },
> > -		{ "basic-store-each", store_ring, 1, 2 },
> > -		{ "basic-many-each", store_many, 0, 2 },
> > -		{ "switch-each", switch_ring, 1, 20 },
> > -		{ "forked-switch-each", switch_ring, ncpus, 20 },
> > -		{ "forked-each", sync_ring, ncpus, 20 },
> > -		{ "forked-store-each", store_ring, ncpus, 20 },
> > -		{ "active-each", active_ring, 0, 20 },
> > -		{ "wakeup-each", wakeup_ring, 20, 1 },
> > -		{ "active-wakeup-each", active_wakeup_ring, 20, 1 },
> > -		{ "double-wakeup-each", wakeup_ring, 20, 2 },
> > +		{ "basic-each", sync_ring, 1, MTIMEOUT },
> > +		{ "basic-store-each", store_ring, 1, MTIMEOUT },
> > +		{ "basic-many-each", store_many, 0, MTIMEOUT },
> > +		{ "switch-each", switch_ring, 1, LTIMEOUT },
> > +		{ "forked-switch-each", switch_ring, ncpus, LTIMEOUT },
> > +		{ "forked-each", sync_ring, ncpus, LTIMEOUT },
> > +		{ "forked-store-each", store_ring, ncpus, LTIMEOUT },
> > +		{ "active-each", active_ring, 0, LTIMEOUT },
> > +		{ "wakeup-each", wakeup_ring, 20, STIMEOUT },
> > +		{ "active-wakeup-each", active_wakeup_ring, 20, STIMEOUT },
> > +		{ "double-wakeup-each", wakeup_ring, 20, MTIMEOUT },
> >		{}
> >	}, individual[] = {
> > -		{ "default", sync_ring, 1, 20 },
> > -		{ "idle", idle_ring, 0, 20 },
> > -		{ "active", active_ring, 0, 20 },
> > -		{ "wakeup", wakeup_ring, 20, 1 },
> > -		{ "active-wakeup", active_wakeup_ring, 20, 1 },
> > -		{ "double-wakeup", wakeup_ring, 20, 2 },
> > -		{ "store", store_ring, 1, 20 },
> > -		{ "switch", switch_ring, 1, 20 },
> > -		{ "forked-switch", switch_ring, ncpus, 20 },
> > -		{ "many", store_many, 0, 20 },
> > -		{ "forked", sync_ring, ncpus, 20 },
> > -		{ "forked-store", store_ring, ncpus, 20 },
> > +		{ "default", sync_ring, 1, LTIMEOUT },
> > +		{ "idle", idle_ring, 0, LTIMEOUT },
> > +		{ "active", active_ring, 0, LTIMEOUT },
> > +		{ "wakeup", wakeup_ring, 20, STIMEOUT },
> > +		{ "active-wakeup", active_wakeup_ring, 20, STIMEOUT },
> > +		{ "double-wakeup", wakeup_ring, 20, MTIMEOUT },
> > +		{ "store", store_ring, 1, LTIMEOUT },
> > +		{ "switch", switch_ring, 1, LTIMEOUT },
> > +		{ "forked-switch", switch_ring, ncpus, LTIMEOUT },
> > +		{ "many", store_many, 0, LTIMEOUT },
> > +		{ "forked", sync_ring, ncpus, LTIMEOUT },
> > +		{ "forked-store", store_ring, ncpus, LTIMEOUT },
> >		{}
> >	};
> >  #define for_each_test(t, T) for(typeof(*T) *t = T; t->name; t++)
> > @@ -1215,19 +1220,20 @@ igt_main
> >		}
> >	}
> >
> > +
> >	igt_subtest("basic-all")
> > -		sync_all(fd, ctx, 1, 2);
> > +		sync_all(fd, ctx, 1, MTIMEOUT);
> >	igt_subtest("basic-store-all")
> > -		store_all(fd, ctx, 1, 2);
> > +		store_all(fd, ctx, 1, MTIMEOUT);
> >
> >	igt_subtest("all")
> > -		sync_all(fd, ctx, 1, 20);
> > +		sync_all(fd, ctx, 1, LTIMEOUT);
> >	igt_subtest("store-all")
> > -		store_all(fd, ctx, 1, 20);
> > +		store_all(fd, ctx, 1, LTIMEOUT);
> >	igt_subtest("forked-all")
> > -		sync_all(fd, ctx, ncpus, 20);
> > +		sync_all(fd, ctx, ncpus, LTIMEOUT);
> >	igt_subtest("forked-store-all")
> > -		store_all(fd, ctx, ncpus, 20);
> > +		store_all(fd, ctx, ncpus, LTIMEOUT);
> >
> >	for_each_test(t, all) {
> >		igt_subtest_f("%s", t->name)
> > @@ -1254,11 +1260,11 @@ igt_main
> >		}
> >
> >		igt_subtest("preempt-all")
> > -			preempt(fd, ctx, ALL_ENGINES, 1, 20);
> > +			preempt(fd, ctx, ALL_ENGINES, 1, LTIMEOUT);
> >		igt_subtest_with_dynamic("preempt") {
> >			for_each_ctx_engine(fd, ctx, e) {
> >				igt_dynamic_f("%s", e->name)
> > -					preempt(fd, ctx, e->flags, ncpus, 20);
> > +					preempt(fd, ctx, e->flags, ncpus, LTIMEOUT);
> >			}
> >		}
> >	}
> > --
> > 2.26.0
> >