[v9, 4/4] tests/intel/xe_sysfs_scheduler: Assert sysfs params are restored

[Cavitt, Jonathan]

-----Original Message-----
From: Kamil Konieczny <kamil.konieczny at linux.intel.com> 
Sent: Wednesday, December 4, 2024 10:23 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Cc: igt-dev at lists.freedesktop.org; Gupta, saurabhg <saurabhg.gupta at intel.com>; Zuo, Alex <alex.zuo at intel.com>; Belgaumkar, Vinay <vinay.belgaumkar at intel.com>; Upadhyay, Tejas <tejas.upadhyay at intel.com>
Subject: Re: [v9,4/4] tests/intel/xe_sysfs_scheduler: Assert sysfs params are restored
> 
> On 2024-12-03 at 22:32:13 +0000, Jonathan Cavitt wrote:
> > The xe_sysfs_scheduler tests modify various sysfs parameters.  At the
> > end of the test, the sysfs parameters are restored, but we do not
> > currently assert that the restoration process completes successfully.
> > Assert the restoration is successful.
> > 
> > Additionally, when the tests fail, it is possible that the various
> > modified sysfs parameters may be left in modified states, which can
> > cause future tests to behave unpredictably.  At the end of the test,
> > attempt to restore all modified sysfs parameters to their original
> > values, aborting all tests if this is unsuccessful.
> > 
> > v2:
> > - Store engine lists (Kamil)
> > 
> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> 
> Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Thank you for the reviews!  I just noticed a small formatting error in patch 3 that I'd
like to repair before this is pushed, so give me a moment to fix that and submit the final
revision with those RBs applied.
-Jonathan Cavitt

> 
> +cc Tejas
> 
> > ---
> >  tests/intel/xe_sysfs_scheduler.c | 89 ++++++++++++++++++++++++++++----
> >  1 file changed, 79 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> > index 947dbdbc9b..89c87c2eed 100644
> > --- a/tests/intel/xe_sysfs_scheduler.c
> > +++ b/tests/intel/xe_sysfs_scheduler.c
> > @@ -107,10 +107,19 @@ static void test_min_max(int xe, int engine, const char **property,
> >  
> >  	/* Reset property, max, min to original values */
> >  	igt_sysfs_printf(engine, property[0], "%d", store);
> > +	igt_sysfs_scanf(engine, property[0], "%u", &set);
> > +	igt_assert_eq(set, store);
> > +
> >  	igt_sysfs_printf(engine, property[1], "%d", default_min);
> > +	igt_sysfs_scanf(engine, property[1], "%u", &set);
> > +	igt_assert_eq(set, default_min);
> > +
> >  	igt_sysfs_printf(engine, property[2], "%d", default_max);
> > +	igt_sysfs_scanf(engine, property[2], "%u", &set);
> > +	igt_assert_eq(set, default_max);
> >  }
> >  
> > +#define MAX_GTS 8
> >  igt_main
> >  {
> >  	static const struct {
> > @@ -126,10 +135,15 @@ igt_main
> >  				      {"timeslice_duration_us", "timeslice_duration_min", "timeslice_duration_max"},
> >  				      {"job_timeout_ms", "job_timeout_min", "job_timeout_max"},
> >  	};
> > +
> > +	unsigned int store[MAX_GTS][3][3];
> >  	int count = sizeof(property) / sizeof(property[0]);
> > +	int gt_count = 0;
> >  	int xe = -1;
> >  	int sys_fd;
> >  	int gt;
> > +	int engines_fd[MAX_GTS], gt_fd[MAX_GTS];
> > +	int *engine_list[MAX_GTS];
> >  
> >  	igt_fixture {
> >  		xe = drm_open_driver(DRIVER_XE);
> > @@ -138,28 +152,83 @@ igt_main
> >  		sys_fd = igt_sysfs_open(xe);
> >  		igt_require(sys_fd != -1);
> >  		close(sys_fd);
> > +
> > +		xe_for_each_gt(xe, gt) {
> > +			int *list, i = 0;
> > +
> > +			igt_require(gt_count < MAX_GTS);
> > +
> > +			gt_fd[gt_count] = xe_sysfs_gt_open(xe, gt);
> > +			igt_require(gt_fd[gt_count] != -1);
> > +			engines_fd[gt_count] = openat(gt_fd[gt_count], "engines", O_RDONLY);
> > +			igt_require(engines_fd[gt_count] != -1);
> > +
> > +			list = igt_sysfs_get_engine_list(engines_fd[gt_count]);
> > +
> > +			while (list[i] != -1) {
> > +				for (int j = 0; j < count; j++) {
> > +					const char **pl = property[j];
> > +
> > +					for (int k = 0; k < 3; k++) {
> > +						unsigned int *loc = &store[i][j][k];
> > +
> > +						igt_require(igt_sysfs_scanf(list[i], pl[k],
> > +									    "%u", loc) == 1);
> > +					}
> > +				}
> > +				i++;
> > +			}
> > +
> > +			igt_require(i > 0);
> > +			engine_list[gt_count] = list;
> > +			gt_count++;
> > +		}
> >  	}
> >  
> >  	for (int i = 0; i < count; i++) {
> >  		for (typeof(*tests) *t = tests; t->name; t++) {
> >  			igt_subtest_with_dynamic_f("%s-%s", property[i][0], t->name) {
> > +				int j = 0;
> >  				xe_for_each_gt(xe, gt) {
> > -					int engines_fd = -1;
> > -					int gt_fd = -1;
> > -
> > -					gt_fd = xe_sysfs_gt_open(xe, gt);
> > -					igt_require(gt_fd != -1);
> > -					engines_fd = openat(gt_fd, "engines", O_RDONLY);
> > -					igt_require(engines_fd != -1);
> > +					int e = engines_fd[j];
> >  
> > -					igt_sysfs_engines(xe, engines_fd, 0, 0, property[i], t->fn);
> > -					close(engines_fd);
> > -					close(gt_fd);
> > +					igt_sysfs_engines(xe, e, 0, 0, property[i], t->fn);
> > +					j++;
> >  				}
> >  			}
> >  		}
> >  	}
> > +
> >  	igt_fixture {
> > +		for (int gtn = gt_count - 1; gtn >= 0; gtn--) {
> > +			int *list, i = 0;
> > +
> > +			list = engine_list[gtn];
> > +
> > +			while (list[i] != -1) {
> > +				int e = list[i];
> > +
> > +				for (int j = count - 1; j >= 0; j--) {
> > +					const char **pl = property[j];
> > +
> > +					for (int k = 2; k >= 0; k--) {
> > +						unsigned int read = UINT_MAX;
> > +						unsigned int val = store[i][j][k];
> > +
> > +						igt_sysfs_printf(e, pl[k], "%u", val);
> > +						igt_sysfs_scanf(e, pl[k], "%u", &read);
> > +						igt_abort_on_f(read != val,
> > +							       "%s not restored!\n", pl[k]);
> > +					}
> > +				}
> > +				i++;
> > +			}
> > +
> > +			igt_sysfs_free_engine_list(list);
> > +			close(engines_fd[gtn]);
> > +			close(gt_fd[gtn]);
> > +		}
> > +
> >  		xe_device_put(xe);
> >  		close(xe);
> >  	}
>