Re: drm/xe: xe_hw_engine_class: Add fault injection and improve error handling in kobj_xe_hw_engine_class

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

Hi,

Please use shorter title

You may drop "xe_hw_engine_class:" tag from the prefix
Just "drm/xe:" or "drm/xe/sysfs:" should be sufficient

On 05.04.2025 09:17, Sobin Thomas wrote:
> - Added fault injection for `kobj_xe_hw_engine_class` to enhance coverage.
> - Improved error handling by checking for `IS_ERR_OR_NULL` on `keclass` and
>   propagating the error using `PTR_ERR`.

Describe your changes according to [1] and (in case it turns out both
changes are correct) consider split them into two patches since they are
logically separate [2]

[1]
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

[2]
https://docs.kernel.org/process/submitting-patches.html#separate-your-changes

> 
> Can be tested using inject-fault-probe-function-kobj_xe_hw_engine_class

Please mention that this is about IGT test (or use igt@... notation)

> 
> Signed-off-by: Sobin Thomas <sobin.thomas@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> index 640950172088..0cafe1175389 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> @@ -571,6 +571,7 @@ kobj_xe_hw_engine_class(struct xe_device *xe, struct kobject *parent, const char
>  
>  	return keclass;
>  }
> +ALLOW_ERROR_INJECTION(kobj_xe_hw_engine_class, NULL);
>  
>  static void hw_engine_class_defaults_fini(void *arg)
>  {
> @@ -662,8 +663,8 @@ int xe_hw_engine_class_sysfs_init(struct xe_gt *gt)
>  		}
>  
>  		keclass = kobj_xe_hw_engine_class(xe, kobj, name);
> -		if (!keclass) {
> -			err = -EINVAL;
> +		if (IS_ERR_OR_NULL(keclass)) {

why?

kobj_xe_hw_engine_class() seems to be always returning NULL on any error
condition, and for error injection cases you also declare it as
EI_ETYPE_NULL, not as EI_ETYPE_ERRNO_NULL like this condition may
suggest, so it should be never an ERR_PTR

> +			err = PTR_ERR(keclass);

and now when kobj_xe_hw_engine_class() actually fails and returns NULL
then "err" will be 0 instead of any proper error code

>  			goto err_object;
>  		}
>