Re: [PATCH i-g-t] tests/xe_exec_threads: Fill in GT field for second balancer thread

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Mar 20, 2024 at 03:47:57PM -0500, Lucas De Marchi wrote:
> On Wed, Mar 20, 2024 at 12:23:56PM -0700, Matt Roper wrote:
> > The balancer subtests spawn two pthreads per engine class if there are
> > multiple instances of the class.  The GT field of the data structure is
> > filled in properly for the first thread, but not for the second,
> > effectively leaving it set to "0."
> > 
> > For platforms with standalone media, this will result in failures when
> > the thread tries to find the instances of a media class on GT0 and trips
> > the "igt_assert(num_placements > 1)" assertion in test_balancer().
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> 
> 
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> do you know why we unrolled a loop in there and did a wrong copy and

I think the for_gt loop was added after the original version of this.
Likely an ommision when that was added.

> paste? That together with vertical spacing on flow control seems to be a
> good source of bugs. Good find.
> 
> The only difference between the first and second thread data I'm seeing
> is flags VIRTUAL vs PARALLEL. Matt Brost, is this test from you? Could

Yes, I wrote this one. Not my finest work in terms of copy / pasting,
indeed the only difference should be VIRTUAL vs. PARALLEL. I think this
should be consolidated into a loop.

> you add an overview to its documentation? The boilerplate doc added
> there after the fact looks less than helpful.
>

What kinda of documentation were you thinking of? Most all the IGTs are
light on doc but updating the doc for ones I wrote is on my todo list...

Matt
 
> Lucas De Marchi
> 
> > ---
> > tests/intel/xe_exec_threads.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/intel/xe_exec_threads.c b/tests/intel/xe_exec_threads.c
> > index 55907e2b3..977e8c600 100644
> > --- a/tests/intel/xe_exec_threads.c
> > +++ b/tests/intel/xe_exec_threads.c
> > @@ -1081,6 +1081,7 @@ static void threads(int fd, int flags)
> > 						threads_data[i].fd = 0;
> > 					else
> > 						threads_data[i].fd = fd;
> > +					threads_data[i].gt = gt;
> > 					threads_data[i].vm_legacy_mode =
> > 						vm_legacy_mode;
> > 					threads_data[i].class = class;
> > -- 
> > 2.43.0
> >