Re: [igt-dev] [PATCH i-g-t 6/7] benchmarks/gem_exec_fault: Add timeout argument

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Thu, 14 Oct 2021 00:47:39 -0700, Zbigniew Kempczyński wrote:
> On Wed, Oct 13, 2021 at 09:24:18PM -0700, Dixit, Ashutosh wrote:
> > On Thu, 07 Oct 2021 23:54:31 -0700, Zbigniew Kempczyński wrote:
> > >
> > > +static int loop(uint64_t size, unsigned ring, int reps, int ncpus,
> > > +		unsigned flags, float timeout)
> > >  {
> > >	struct drm_i915_gem_execbuffer2 execbuf;
> > >	struct drm_i915_gem_exec_object2 obj;
> > > @@ -128,10 +130,14 @@ static int loop(uint64_t size, unsigned ring, int reps, int ncpus, unsigned flag
> > >					/* fault out */
> > >					obj.alignment = 1ull << 63;
> > >					__gem_execbuf(fd, &execbuf);
> > > -				}
> > >
> > > -				clock_gettime(CLOCK_MONOTONIC, &end);
> > > -			} while (elapsed(&start, &end) < 2.);
> > > +					clock_gettime(CLOCK_MONOTONIC, &end);
> > > +					if (elapsed(&start, &end) >= timeout) {
> > > +						timeout = -1.0;
> >
> > Just a small nit, I don't think we need to set it to -1 here. We can just
> > leave the "while (elapsed(&start, &end) < timeout);" check in the outer loop
> > too. So please change if you agree. Otherwise:
>
> I just wanted to avoid calling elapsed() twice, that's why I changed to
> simpler expression. Timeout cannot be negative so it is good as stop
> condition to outer loop.
>
> I dare get your r-b here.

Sure, just a matter of personal preference. Already R-b.

>
> --
> Zbigniew
>
> >
> > Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >
> > > +						break;
> > > +					}
> > > +				}
> > > +			} while (timeout > 0);
> > >
> > >			gem_sync(fd, obj.handle);
> > >			clock_gettime(CLOCK_MONOTONIC, &end);