From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id E97816EC1B for ; Thu, 14 Oct 2021 07:49:58 +0000 (UTC) Date: Thu, 14 Oct 2021 00:30:43 -0700 Message-ID: <87v920gj18.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20211014074739.GD3617@zkempczy-mobl2> References: <20211008065432.15482-1-zbigniew.kempczynski@intel.com> <20211008065432.15482-7-zbigniew.kempczynski@intel.com> <8735p4i68d.wl-ashutosh.dixit@intel.com> <20211014074739.GD3617@zkempczy-mobl2> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=ISO-8859-2 Content-Transfer-Encoding: quoted-printable Subject: Re: [igt-dev] [PATCH i-g-t 6/7] benchmarks/gem_exec_fault: Add timeout argument List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Zbigniew =?ISO-8859-2?Q?Kempczy=F1ski?= Cc: igt-dev@lists.freedesktop.org, Petri Latvala List-ID: On Thu, 14 Oct 2021 00:47:39 -0700, Zbigniew Kempczy=F1ski 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=F1ski 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, i= nt reps, int ncpus, unsigned flag > > > /* fault out */ > > > obj.alignment =3D 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) >=3D timeout) { > > > + timeout =3D -1.0; > > > > Just a small nit, I don't think we need to set it to -1 here. We can ju= st > > 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 > > > > > + break; > > > + } > > > + } > > > + } while (timeout > 0); > > > > > > gem_sync(fd, obj.handle); > > > clock_gettime(CLOCK_MONOTONIC, &end);