[PATCH v8 4/4] tests/intel/xe_sysfs_scheduler: Assert sysfs params are restored

[Kamil Konieczny]

Hi Jonathan,
On 2024-12-02 at 18:08:16 +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.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> ---
>  tests/intel/xe_sysfs_scheduler.c | 88 ++++++++++++++++++++++++++++----
>  1 file changed, 78 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/intel/xe_sysfs_scheduler.c b/tests/intel/xe_sysfs_scheduler.c
> index 947dbdbc9b..7b7cd960c6 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,14 @@ 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];
>  
>  	igt_fixture {
>  		xe = drm_open_driver(DRIVER_XE);
> @@ -138,28 +151,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;

List should be global in igt_main.

> +
> +			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);
> +			igt_sysfs_free_engine_list(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 = igt_sysfs_get_engine_list(engines_fd[gtn]);

It should be filled in first igt_fixture and removed from this.
With this fixed you could add my r-b.

Regards,
Kamil

> +
> +			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);
>  	}
> -- 
> 2.43.0
>