[PATCH v3 1/2] tests/intel/sysfs: Restore sysfs values on test failure

Kamil Konieczny kamil.konieczny at linux.intel.com
Tue Nov 5 17:49:46 UTC 2024


Hi Jonathan,
On 2024-11-05 at 15:48:22 +0000, Jonathan Cavitt wrote:

please add 'xe_' in subject, instead of:

[PATCH v3 1/2] tests/intel/sysfs: Restore sysfs values on test failure

better:

[PATCH v3 1/2] tests/intel/xe_sysfs*: Restore values on test failure

> The tests xe_sysfs_preempt_timeout and xe_sysfs_timeslice_duration
> modify the values of preempt_timeout_us and timeslice_duration_us,
> respectively.  However, on a test failure, it is possible that these
> values may remain in their modified states, resulting in the values
> being used in future tests and causing unexpected behavior.
> 
> Save the respective modified values before starting the test and attempt
> to restore the values on test exit.
> 
> v2:
> - Fix some formatting issues (Kamil)
> - Abort if value restore fails (Kamil)
> - Directly call igt_sysfs_printf on exit to avoid duplicating on helper
>   (Kamil)
> 
> Suggested-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
> CC: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> ---
>  tests/intel/xe_sysfs_preempt_timeout.c    | 43 +++++++++++++++++------
>  tests/intel/xe_sysfs_timeslice_duration.c | 42 +++++++++++++++++-----
>  2 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/tests/intel/xe_sysfs_preempt_timeout.c b/tests/intel/xe_sysfs_preempt_timeout.c
> index 7fa0dfcdf7..3bfda826d4 100644
> --- a/tests/intel/xe_sysfs_preempt_timeout.c
> +++ b/tests/intel/xe_sysfs_preempt_timeout.c
> @@ -170,6 +170,7 @@ static void test_timeout(int fd, int engine, const char **property, uint16_t cla
>  	set_preempt_timeout(engine, saved);
>  }
>  
> +#define	MAX_GTS	8
>  igt_main
>  {
>  	static const struct {
> @@ -183,8 +184,10 @@ igt_main
>  				       "preempt_timeout_min",
>  				       "preempt_timeout_max"}, };
>  	int count = sizeof(property) / sizeof(property[0]);
> +	int gt_count = 0;
>  	int fd = -1, sys_fd, gt;
> -	int engines_fd = -1, gt_fd = -1;
> +	int engines_fd[MAX_GTS], gt_fd[MAX_GTS];
> +	unsigned int pts[MAX_GTS];
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_XE);
> @@ -192,26 +195,46 @@ igt_main
>  		sys_fd = igt_sysfs_open(fd);
>  		igt_require(sys_fd != -1);
>  		close(sys_fd);
> +
> +		xe_for_each_gt(fd, gt) {
> +			igt_require(gt_count < MAX_GTS);
> +
> +			gt_fd[gt_count] = xe_sysfs_gt_open(fd, 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);
> +			igt_require(igt_sysfs_scanf(engines_fd[gt_count],
> +						    "preempt_timeout_us",
> +						    "%u", &pts[gt_count]) == 1);
> +			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(fd, gt) {
> -					gt_fd = xe_sysfs_gt_open(fd, gt);
> -					igt_require(gt_fd != -1);
> -					engines_fd = openat(gt_fd, "engines", O_RDONLY);
> -					igt_require(engines_fd != -1);
> -
> -					igt_sysfs_engines(fd, engines_fd, gt, 1, property[i],
> -									  t->fn);
> -					close(engines_fd);
> -					close(gt_fd);
> +					int e = engines_fd[j];

Add newline.

> +					igt_sysfs_engines(fd, e, gt, 1, property[i], t->fn);
> +					j++;
>  				}
>  			}
>  		}
>  	}
>  	igt_fixture {
> +		for (int i = gt_count - 1; i >= 0; i--) {
> +			int store;
> +
> +			igt_assert_lte(0, igt_sysfs_printf(engines_fd[i], "preempt_timeout_us",
> +							   "%u", pts[i]));
> +			igt_sysfs_scanf(engines_fd[i], "preempt_timeout_us", "%u", &store);
> +			igt_abort_on_f(store != pts[i], "preempt_timeout_us not restored!\n");
> +
> +			close(engines_fd[i]);
> +			close(gt_fd[i]);
> +		}
> +
>  		drm_close_driver(fd);
>  	}
>  }
> diff --git a/tests/intel/xe_sysfs_timeslice_duration.c b/tests/intel/xe_sysfs_timeslice_duration.c
> index cf95a3ac1c..b34d78a784 100644
> --- a/tests/intel/xe_sysfs_timeslice_duration.c
> +++ b/tests/intel/xe_sysfs_timeslice_duration.c
> @@ -142,6 +142,7 @@ static void test_timeout(int fd, int engine, const char **property, uint16_t cla
>  	set_timeslice_duration(engine, saved);
>  }
>  
> +#define	MAX_GTS	8
>  igt_main
>  {
>  	static const struct {
> @@ -155,8 +156,10 @@ igt_main
>  				       "timeslice_duration_min",
>  				       "timeslice_duration_max"}, };
>  	int count = sizeof(property) / sizeof(property[0]);
> +	int gt_count = 0;
>  	int fd = -1, sys_fd, gt;
> -	int engines_fd = -1, gt_fd = -1;
> +	int engines_fd[MAX_GTS], gt_fd[MAX_GTS];
> +	unsigned int tds[MAX_GTS];
>  
>  	igt_fixture {
>  		fd = drm_open_driver(DRIVER_XE);
> @@ -164,25 +167,46 @@ igt_main
>  		sys_fd = igt_sysfs_open(fd);
>  		igt_require(sys_fd != -1);
>  		close(sys_fd);
> +
> +		xe_for_each_gt(fd, gt) {
> +			igt_require(gt_count < MAX_GTS);
> +
> +			gt_fd[gt_count] = xe_sysfs_gt_open(fd, 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);
> +			igt_require(igt_sysfs_scanf(engines_fd[gt_count],
> +						    "timeslice_duration_us",
> +						    "%u", &tds[gt_count]) == 1);
> +			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(fd, gt) {
> -					gt_fd = xe_sysfs_gt_open(fd, gt);
> -					igt_require(gt_fd != -1);
> -					engines_fd = openat(gt_fd, "engines", O_RDONLY);
> -					igt_require(engines_fd != -1);
> -					igt_sysfs_engines(fd, engines_fd, gt, 1, property[i],
> -										 t->fn);
> -					close(engines_fd);
> -					close(gt_fd);
> +					int e = engines_fd[j];

Add newline.

With above fixed
Reviewed-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> +					igt_sysfs_engines(fd, e, gt, 1, property[i], t->fn);
> +					j++;
>  				}
>  			}
>  		}
>  	}
>  	igt_fixture {
> +		for (int i = gt_count - 1; i >= 0; i--) {
> +			unsigned int store;
> +
> +			igt_assert_lte(0, igt_sysfs_printf(engines_fd[i], "timeslice_duration_us",
> +							   "%u", tds[i]));
> +			igt_sysfs_scanf(engines_fd[i], "timeslice_duration_us", "%u", &store);
> +			igt_abort_on_f(store != tds[i], "timeslice_duration_us not restored!\n");
> +
> +			close(engines_fd[i]);
> +			close(gt_fd[i]);
> +		}
> +
>  		drm_close_driver(fd);
>  	}
>  }
> -- 
> 2.43.0
> 


More information about the igt-dev mailing list