[PATCH i-g-t] tests/xe_create: Use separate VMs per process

[Cavitt, Jonathan]

-----Original Message-----
From: Kempczynski, Zbigniew <zbigniew.kempczynski at intel.com> 
Sent: Monday, December 18, 2023 8:49 PM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Cc: igt-dev at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Welty, Brian <brian.welty at intel.com>; kamil.konieczny at linux.intel.com; De Marchi, Lucas <lucas.demarchi at intel.com>; Vishwanathapura, Niranjana <niranjana.vishwanathapura at intel.com>
Subject: Re: [PATCH i-g-t] tests/xe_create: Use separate VMs per process
> 
> On Mon, Dec 18, 2023 at 06:50:51AM -0800, Jonathan Cavitt wrote:
> > We currently exercise cross-user impact in xe_create by attempting to
> > concurrently create a large number of exec queues on a single vm using
> > forked child processes.  This is unrealistic, as multiple users are more
> > likely to be using separate file descriptors for creating exec queues.
> > 
> > Update the test to reflect this use case.
> 
> As original version of the test reveals bug in KMD Brian suggested in offline
> discussion we should keep current test behavior (but not executed in BAT
> with MAXTIME tweaked to relatively large value). I mean we should have four
> subtests - two with current behavior with increased maxtime not executed in
> BAT + two with maxtime set to 5/6. I see ATSM passed with your change but
> it is on the time limit boundary.


I'm sorry.  I don't fully understand what change you're suggesting here.

It sounds like you want me to create a duplicate of the original test that
increases MAXTIME to, say, 15, while also leaving an old version that
leaves MAXTIME at 5.  That's doable, but it also sounds like you want
to run the test with the low MAXTIME on BAT and not the test with
the high MAXTIME.

Doesn't that defeat the purpose of creating a duplicate test, since
we're not changing the test behavior on BAT and thus the issue would
still get reported?  Or did you mean for those tests to be put the other
way around?  Or are you referring to the test change in this patch when
referring to the "two [tests] with maxtime set to 5/6"?  Or am I
misunderstanding what the BAT tests are?

-Jonathan Cavitt


> 
> --
> Zbigniew
> 
> > 
> > Suggested-by: Brian Welty <brian.welty at intel.com>
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> > CC: Zbigniew Kempczynski <zbigniew.kempczynski at intel.com>
> > CC: Lucas De Marchi <lucas.demarchi at intel.com>
> > CC: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > CC: Niranjana Vishwanathapura <niranjana.vishwanathapura at intel.com>
> > ---
> >  tests/intel/xe_create.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tests/intel/xe_create.c b/tests/intel/xe_create.c
> > index 0aa32c788a..22f9464161 100644
> > --- a/tests/intel/xe_create.c
> > +++ b/tests/intel/xe_create.c
> > @@ -167,13 +167,9 @@ enum exec_queue_destroy {
> >  static void create_execqueues(int fd, enum exec_queue_destroy ed)
> >  {
> >  	struct timespec tv = { };
> > -	uint32_t num_engines, exec_queues_per_process, vm;
> > +	uint32_t exec_queues_per_process;
> >  	int nproc = sysconf(_SC_NPROCESSORS_ONLN), seconds;
> >  
> > -	fd = drm_reopen_driver(fd);
> > -	num_engines = xe_number_engines(fd);
> > -	vm = xe_vm_create(fd, 0, 0);
> > -
> >  	exec_queues_per_process = max_t(uint32_t, 1, MAXEXECQUEUES / nproc);
> >  	igt_debug("nproc: %u, exec_queues per process: %u\n", nproc, exec_queues_per_process);
> >  
> > @@ -182,8 +178,13 @@ static void create_execqueues(int fd, enum exec_queue_destroy ed)
> >  	igt_fork(n, nproc) {
> >  		struct drm_xe_engine *engine;
> >  		uint32_t exec_queue, exec_queues[exec_queues_per_process];
> > +		uint32_t num_engines, vm;
> >  		int idx, err, i;
> >  
> > +		fd = drm_reopen_driver(fd);
> > +		num_engines = xe_number_engines(fd);
> > +		vm = xe_vm_create(fd, 0, 0);
> > +
> >  		srandom(n);
> >  
> >  		for (i = 0; i < exec_queues_per_process; i++) {
> > @@ -206,12 +207,12 @@ static void create_execqueues(int fd, enum exec_queue_destroy ed)
> >  				xe_exec_queue_destroy(fd, exec_queues[i]);
> >  			}
> >  		}
> > +
> > +		xe_vm_destroy(fd, vm);
> > +		drm_close_driver(fd);
> >  	}
> >  	igt_waitchildren();
> >  
> > -	xe_vm_destroy(fd, vm);
> > -	drm_close_driver(fd);
> > -
> >  	seconds = igt_seconds_elapsed(&tv);
> >  	igt_assert_f(seconds < MAXTIME,
> >  		     "Creating %d exec_queues tooks too long: %d [limit: %d]\n",
> > -- 
> > 2.25.1
> > 
>