Re: [PATCH i-g-t] tests/intel/xe_fault_injection: Use separate functions

["Pottumuttu, Sai Teja" <sai.teja.pottumuttu@intel.com>]

On 25-03-2025 19:21, Lucas De Marchi wrote:
> Do not mux everything in a _do() function, but rather use
> _add()/_remove()/_clear() functions for clarity and brevity.
> 
> The add()/remove() boilerplate is small enough to just be repeated. For
> clear(), just implement it using add().
> 
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

There is a new fault injection test added recently, 
oa-add-config-fail-*. So, this patch just needs a rebase and the same 
change applied to the new test.

Other than that, looks good.

Reviewed-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>

> ---
>   tests/intel/xe_fault_injection.c | 55 +++++++++++++++++---------------
>   1 file changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/tests/intel/xe_fault_injection.c b/tests/intel/xe_fault_injection.c
> index 1bd0f51a3..dcbe0e406 100644
> --- a/tests/intel/xe_fault_injection.c
> +++ b/tests/intel/xe_fault_injection.c
> @@ -26,11 +26,6 @@
>   #define BO_ADDR		0x1a0000
>   #define BO_SIZE		(1024*1024)
>   
> -enum injection_list_action {
> -	INJECTION_LIST_ADD,
> -	INJECTION_LIST_REMOVE,
> -};
> -
>   static int fail_function_open(void)
>   {
>   	int debugfs_fail_function_dir_fd;
> @@ -96,27 +91,36 @@ static bool fail_function_injection_enabled(void)
>   	return true;
>   }
>   
> -static void injection_list_do(enum injection_list_action action, const char function_name[])
> +static void injection_list_add(const char function_name[])
>   {
>   	int dir;
>   
>   	dir = fail_function_open();
> +
>   	igt_assert_lte(0, dir);
> +	igt_assert_lte(0, igt_sysfs_printf(dir, "inject", "%s", function_name));
>   
> -	switch(action) {
> -	case INJECTION_LIST_ADD:
> -		igt_assert_lte(0, igt_sysfs_printf(dir, "inject", "%s", function_name));
> -		break;
> -	case INJECTION_LIST_REMOVE:
> -		igt_assert_lte(0, igt_sysfs_printf(dir, "inject", "!%s", function_name));
> -		break;
> -	default:
> -		igt_assert(!"missing");
> -	}
> +	close(dir);
> +}
> +
> +static void injection_list_remove(const char function_name[])
> +{
> +	int dir;
> +
> +	dir = fail_function_open();
> +
> +	igt_assert_lte(0, dir);
> +	igt_assert_lte(0, igt_sysfs_printf(dir, "inject", "!%s", function_name));
>   
>   	close(dir);
>   }
>   
> +static void injection_list_clear(void)
> +{
> +	/* If nothing specified (‘’) injection list is cleared */
> +	return injection_list_add("");
> +}
> +
>   /*
>    * See https://docs.kernel.org/fault-injection/fault-injection.html#application-examples
>    */
> @@ -139,8 +143,7 @@ static void setup_injection_fault(void)
>   
>   static void cleanup_injection_fault(int sig)
>   {
> -	/* If nothing specified (‘’) injection list is cleared */
> -	injection_list_do(INJECTION_LIST_ADD, "");
> +	injection_list_clear();
>   }
>   
>   static void set_retval(const char function_name[], long long retval)
> @@ -185,11 +188,11 @@ inject_fault_probe(int fd, char pci_slot[], const char function_name[])
>   		 strerror(-INJECT_ERRNO), INJECT_ERRNO, function_name);
>   
>   	ignore_faults_in_dmesg(function_name);
> -	injection_list_do(INJECTION_LIST_ADD, function_name);
> +	injection_list_add(function_name);
>   	set_retval(function_name, INJECT_ERRNO);
>   	xe_sysfs_driver_do(fd, pci_slot, XE_SYSFS_DRIVER_TRY_BIND);
>   	igt_assert_eq(-errno, INJECT_ERRNO);
> -	injection_list_do(INJECTION_LIST_REMOVE, function_name);
> +	injection_list_remove(function_name);
>   }
>   
>   /**
> @@ -214,10 +217,10 @@ exec_queue_create_fail(int fd, struct drm_xe_engine_class_instance *instance,
>   	xe_exec_queue_destroy(fd, exec_queue_id);
>   
>   	ignore_faults_in_dmesg(function_name);
> -	injection_list_do(INJECTION_LIST_ADD, function_name);
> +	injection_list_add(function_name);
>   	set_retval(function_name, INJECT_ERRNO);
>   	igt_assert(__xe_exec_queue_create(fd, vm, 1, 1, instance, 0, &exec_queue_id) != 0);
> -	injection_list_do(INJECTION_LIST_REMOVE, function_name);
> +	injection_list_remove(function_name);
>   
>   	igt_assert_eq(__xe_exec_queue_create(fd, vm, 1, 1, instance, 0, &exec_queue_id), 0);
>   	xe_exec_queue_destroy(fd, exec_queue_id);
> @@ -249,10 +252,10 @@ vm_create_fail(int fd, const char function_name[], unsigned int flags)
>   	igt_assert_eq(simple_vm_create(fd, flags), 0);
>   
>   	ignore_faults_in_dmesg(function_name);
> -	injection_list_do(INJECTION_LIST_ADD, function_name);
> +	injection_list_add(function_name);
>   	set_retval(function_name, INJECT_ERRNO);
>   	igt_assert(simple_vm_create(fd, flags) != 0);
> -	injection_list_do(INJECTION_LIST_REMOVE, function_name);
> +	injection_list_remove(function_name);
>   
>   	igt_assert_eq(simple_vm_create(fd, flags), 0);
>   }
> @@ -312,10 +315,10 @@ vm_bind_fail(int fd, const char function_name[])
>   	igt_assert_eq(simple_vm_bind(fd, vm), 0);
>   
>   	ignore_faults_in_dmesg(function_name);
> -	injection_list_do(INJECTION_LIST_ADD, function_name);
> +	injection_list_add(function_name);
>   	set_retval(function_name, INJECT_ERRNO);
>   	igt_assert(simple_vm_bind(fd, vm) != 0);
> -	injection_list_do(INJECTION_LIST_REMOVE, function_name);
> +	injection_list_remove(function_name);
>   
>   	igt_assert_eq(simple_vm_bind(fd, vm), 0);
>   }