From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTPS id C314D6EB5D for ; Thu, 5 Aug 2021 21:14:13 +0000 (UTC) Date: Thu, 05 Aug 2021 14:14:12 -0700 Message-ID: <87czqrtx0b.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210805080241.GE3295@zkempczy-mobl2> References: <20210726200026.4815-1-zbigniew.kempczynski@intel.com> <20210726200026.4815-11-zbigniew.kempczynski@intel.com> <87a6lwtziq.wl-ashutosh.dixit@intel.com> <20210805080241.GE3295@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 v3 10/52] tests/gem_busy: Adopt to use allocator 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, 05 Aug 2021 01:02:41 -0700, Zbigniew Kempczy=F1ski wrote: > Please fix the put_ahnd. With that this patch is: Reviewed-by: Ashutosh Dixit But I want to discuss the intel_allocator_multiprocess_start/stop issue a bit more below. > On Wed, Aug 04, 2021 at 07:07:41PM -0700, Dixit, Ashutosh wrote: > > On Mon, 26 Jul 2021 12:59:44 -0700, Zbigniew Kempczy=F1ski wrote: > > > > > > For newer gens we're not able to rely on relocations. Adopt to use > > > offsets acquired from the allocator. > > > > > > Signed-off-by: Zbigniew Kempczy=F1ski > > > Cc: Petri Latvala > > > Cc: Ashutosh Dixit > > > --- > > > tests/i915/gem_busy.c | 35 +++++++++++++++++++++++++++++++---- > > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > > > diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c > > > index f0fca0e8a..51ec5ad04 100644 > > > --- a/tests/i915/gem_busy.c > > > +++ b/tests/i915/gem_busy.c > > > @@ -108,6 +108,7 @@ static void semaphore(int fd, const intel_ctx_t *= ctx, > > > uint32_t handle[3]; > > > uint32_t read, write; > > > uint32_t active; > > > + uint64_t ahnd =3D get_reloc_ahnd(fd, ctx->id); > > > unsigned i; > > > > > > handle[TEST] =3D gem_create(fd, 4096); > > > @@ -117,6 +118,7 @@ static void semaphore(int fd, const intel_ctx_t *= ctx, > > > /* Create a long running batch which we can use to hog the GPU */ > > > handle[BUSY] =3D gem_create(fd, 4096); > > > spin =3D igt_spin_new(fd, > > > + .ahnd =3D ahnd, > > > .ctx =3D ctx, > > > .engine =3D e->flags, > > > .dependency =3D handle[BUSY]); > > > > Missing put_ahnd. > > Good catch. > > > > > > @@ -428,6 +442,7 @@ igt_main > > > > > > igt_subtest_group { > > > igt_fixture { > > > + intel_allocator_multiprocess_start(); > > > igt_fork_hang_detector(fd); > > > } > > > > > > @@ -445,6 +460,21 @@ igt_main > > > } > > > } > > > > > > > Just above here is basic() which doesn't have a fork. Is it ok to do > > intel_allocator_multiprocess_start/stop when we don't have a fork? If y= es, > > then can we _always_ do intel_allocator_multiprocess_start/stop rather = than > > only when we have fork? Thanks. > > intel_allocator_multiprocess_start() creates allocator thread which acts > for children (igt_fork) to alloc/free offsets. If you use alloc/free with= in > same process (from which thread was spawned) internal structure is mutexed > and no IPCs are called. So only consequence of this here is additional th= read > in system/memory (which does nothing for basic() tests). It will be stopp= ed > with intel_allocator_multiprocess_stop(). OK, in that case let me ask the question I asked above in another way. Can we add intel_allocator_multiprocess_start() to common_init() (the program entry point) and similarly say intel_allocator_multiprocess_stop() to igt_exit() (or common_exit_handler(), basically the program exit point) so that these always run and we don't have to add them only for specific tests which fork? What would be the disadvantage of doing this? Thanks. > But for purity test should work without additional dependencies so I'll f= ix > this - it will be sent in v4. > > -- > Zbigniew