Re: [PATCH v4 01/11] drm/xe: Simplify module initialization code

[John Harrison <john.c.harrison@intel.com>]

On 7/27/2025 10:19 AM, Michal Wajdeczko wrote:
> There is no need to have extra checks and WARN() in the helpers
> as instead of an index of the entry with function pointers, we
> can pass pointer to the entry which we prepare directly in the
> main loop, that is guaranteed to be valid.
Not sure this counts as specific to configfs but it looks like a good 
clean up.

>
>    add/remove: 0/0 grow/shrink: 0/4 up/down: 0/-180 (-180)
>    Function                                     old     new   delta
>    xe_exit                                      109      79     -30
>    cleanup_module                               109      79     -30
>    xe_init                                      248     188     -60
>    init_module                                  248     188     -60
>    Total: Before=2774145, After=2773965, chg -0.01%
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_module.c | 27 ++++++++++-----------------
>   1 file changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
> index d9391bd08194..593bc9e5851a 100644
> --- a/drivers/gpu/drm/xe/xe_module.c
> +++ b/drivers/gpu/drm/xe/xe_module.c
> @@ -135,24 +135,17 @@ static const struct init_funcs init_funcs[] = {
>   	},
>   };
>   
> -static int __init xe_call_init_func(unsigned int i)
> +static int __init xe_call_init_func(const struct init_funcs *func)
>   {
> -	if (WARN_ON(i >= ARRAY_SIZE(init_funcs)))
> -		return 0;
> -	if (!init_funcs[i].init)
> -		return 0;
> -
> -	return init_funcs[i].init();
> +	if (func->init)
> +		return func->init();
> +	return 0;
>   }
>   
> -static void xe_call_exit_func(unsigned int i)
> +static void xe_call_exit_func(const struct init_funcs *func)
>   {
> -	if (WARN_ON(i >= ARRAY_SIZE(init_funcs)))
> -		return;
> -	if (!init_funcs[i].exit)
> -		return;
> -
> -	init_funcs[i].exit();
> +	if (func->exit)
> +		func->exit();
>   }
>   
>   static int __init xe_init(void)
> @@ -160,10 +153,10 @@ static int __init xe_init(void)
>   	int err, i;
>   
>   	for (i = 0; i < ARRAY_SIZE(init_funcs); i++) {
> -		err = xe_call_init_func(i);
> +		err = xe_call_init_func(&init_funcs[i]);
Would clearer to write as 'init_funcs + i'. The compiler should generate 
the same code but &x[i] is technically a reference to a derefence of an 
arithmetic operation rather than just an arithmetic operation.

Either way:
Reviewed-by: John Harrison <John.C.Harrison@Intel.com>

>   		if (err) {
>   			while (i--)
> -				xe_call_exit_func(i);
> +				xe_call_exit_func(&init_funcs[i]);
>   			return err;
>   		}
>   	}
> @@ -176,7 +169,7 @@ static void __exit xe_exit(void)
>   	int i;
>   
>   	for (i = ARRAY_SIZE(init_funcs) - 1; i >= 0; i--)
> -		xe_call_exit_func(i);
> +		xe_call_exit_func(&init_funcs[i]);
>   }
>   
>   module_init(xe_init);