Re: [PATCH i-g-t 1/1] tests/intel/xe_eu_stall: Do not check for presence of data on simulation

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

On Tue, 13 May 2025 13:57:41 -0700, Harish Chegondi wrote:
>
> On Tue, May 13, 2025 at 09:43:32AM -0700, Dixit, Ashutosh wrote:
> > On Mon, 12 May 2025 20:07:38 -0700, Harish Chegondi wrote:
> > >
> > > Some simulation models may not have full EU stall sampling support.
> > >
> > > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>
> > > ---
> > >  tests/intel/xe_eu_stall.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/intel/xe_eu_stall.c b/tests/intel/xe_eu_stall.c
> > > index 411c30871..bdfa0fc4b 100644
> > > --- a/tests/intel/xe_eu_stall.c
> > > +++ b/tests/intel/xe_eu_stall.c
> > > @@ -586,7 +586,6 @@ enable:
> > >
> > >	ret = wait_child(&work_load);
> > >	igt_assert_f(ret == 0, "waitpid() - ret: %d, errno: %d\n", ret, errno);
> > > -	igt_assert_f(num_samples, "No EU stalls detected during the workload\n");
> > >
> > >	do_ioctl(stream_fd, DRM_XE_OBSERVATION_IOCTL_DISABLE, 0);
> > >	if (--iter)
> > > @@ -594,6 +593,9 @@ enable:
> > >
> > >	close(stream_fd);
> > >	free(buf);
> > > +
> > > +	if (!igt_run_in_simulation())
> > > +		igt_assert_f(num_samples, "No EU stalls detected during the workload\n");
> >
> > Do we really want to move this here? Wasn't the earlier location better
> > since it checked num_samples for every iteration, whereas now we'd check it
> > only for the last iteration?
> Hi Ashutosh,
>
> Initially I didn't move. When testing I noticed that if there is no
> data, the assert triggers and the following close() and free() are not
> called. When the next sub-test gets executed, it returns EBUSY as the
> stream is not closed in the previous test. So, I moved this check here.
> Anyhow the data from the first iteration is checked in the blocking-read
> and non-blocking-read subtests where there is only one iteration.

Hmm, the problem is, it's making the code look weird now. Also, if the
process dies in an assert, the fd should get closed when the process
died.

Or is there a delay between the process dying and fd getting closed? And
the next process is trying to open the fd before the previous process
closed the fd? Where did you see the EBUSY issue, is it happening in CI? Or
what are you executing to reproduce the EBUSY issue?

Can you please add a print in the eu stall release fops and see if
release() is not getting called when the process dies (put an artificial
assert in igt if needed).

Better to investigate this a little bit more I think.

Thanks.
--
Ashutosh


> >
> >
> > >  }
> > >
> > >  static int opt_handler(int opt, int opt_index, void *data)
> > > --
> > > 2.48.1
> > >