From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id ECE9F10F16F for ; Fri, 8 Apr 2022 21:17:02 +0000 (UTC) Date: Fri, 08 Apr 2022 14:17:01 -0700 Message-ID: <87h7732s36.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Arjun Melkaveri In-Reply-To: <20220408165643.10211-1-arjun.melkaveri@intel.com> References: <20220408165643.10211-1-arjun.melkaveri@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH i-g-t] tests/i915/gem_ctx_persistence: Fixed assert issue seen for heartbeat-* tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: On Fri, 08 Apr 2022 09:56:43 -0700, Arjun Melkaveri wrote: > > heartbeat-* tests were failing with assert mentioned below. Failing where? GuC or non-GuC? If it's GuC maybe tests are supposed to fail since GuC doesn't support persistence properly? > Issue got fixed by moving for_each_physical_ring to > for_each_ctx_cfg_engine, and creating right ctx . > > Issue :- > Stack trace: > #0 ../lib/igt_core.c:1756 __igt_fail_assert() > #1 ../tests/i915/gem_ctx_persistence.c:589 test_noheartbeat_close() There is no assert on this line. > diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c > index 00dda3a8..08cd2a89 100644 > --- a/tests/i915/gem_ctx_persistence.c > +++ b/tests/i915/gem_ctx_persistence.c > @@ -476,8 +476,10 @@ static bool set_preempt_timeout(int i915, const char *name, unsigned int value) > return true; > } > > -static void test_noheartbeat_many(int i915, int count, unsigned int flags) > +static void test_noheartbeat_many(int i915, const intel_ctx_cfg_t *cfg, > + int count, unsigned int flags) > { > + const struct intel_execution_engine2 *e; > unsigned long checked = 0; > > cleanup(i915); > @@ -489,25 +491,25 @@ static void test_noheartbeat_many(int i915, int count, unsigned int flags) > * cleaned up. > */ > > - for_each_physical_ring(e, i915) { > + for_each_ctx_cfg_engine(i915, cfg, e) { But why? With an empty context these two lines are close to equivalent, so why change it? > igt_spin_t *spin[count]; > uint64_t ahnd; > > - if (!set_preempt_timeout(i915, e->full_name, 250)) > + if (!set_preempt_timeout(i915, e->name, 250)) Whey s/full_name/name/ ? > continue; > > - if (!set_heartbeat(i915, e->full_name, 0)) > + if (!set_heartbeat(i915, e->name, 0)) > continue; > > - igt_assert(set_heartbeat(i915, e->full_name, 500)); > + igt_assert(set_heartbeat(i915, e->name, 500)); > > for (int n = 0; n < ARRAY_SIZE(spin); n++) { > const intel_ctx_t *ctx; > > - ctx = intel_ctx_create(i915, NULL); > + ctx = ctx_create_persistence(i915, cfg, false); Just above the comment says this: /* * If the user disables the heartbeat, after leaving behind * a number of long running *persistent* contexts, check they get * cleaned up. */ So now we are changing the context to be non-persistent, thereby changing the test to make it pass? So let's try to first figure out why the tests are failing and what we are trying to fix.