[Intel-xe] [PATCH] drm/xe: Call exit functions when xe_register_pci_driver() fails

[Gustavo Sousa]

Quoting Jani Nikula (2023-05-10 10:22:08)
>On Tue, 09 May 2023, Gustavo Sousa <gustavo.sousa at intel.com> wrote:
>> Make sure all necessary exit functions are also called when
>> xe_register_pci_driver() fails.
>>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_module.c | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
>> index 6860586ce7f8..9698875852d3 100644
>> --- a/drivers/gpu/drm/xe/xe_module.c
>> +++ b/drivers/gpu/drm/xe/xe_module.c
>> @@ -53,14 +53,18 @@ static int __init xe_init(void)
>>  
>>          for (i = 0; i < ARRAY_SIZE(init_funcs); i++) {
>>                  err = init_funcs[i].init();
>> -                if (err) {
>> -                        while (i--)
>> -                                init_funcs[i].exit();
>> -                        return err;
>> -                }
>> +                if (err)
>> +                        break;
>>          }
>>  
>> -        return xe_register_pci_driver();
>> +        if (!err)
>> +                err = xe_register_pci_driver();
>
>Why aren't xe_register_pci_driver() and xe_unregister_pci_driver()
>init/exit functions in the array? It would avoid the special casing
>here, and allow some init functions to be called after register.

Hm... I would imagine that the intention would to make it explicit that
xe_register_pci_driver() must be the latest one called and, if that was the
case (which is probably false based on your comment), maybe adding a comment on
the array entry would also suffice.

So should we just add them to the array?

One issue would to be related to the function names, which would be
incompatible with using MAKE_INIT_EXIT_FUNCS(). Since those functions are only
used in this file, I guess it would be okay to rename them to
xe_pci_module_init() and xe_pci_module_exit() for consistency sake. Thoughts?

--
Gustavo Sousa

>
>BR,
>Jani.
>
>
>> +
>> +        if (err)
>> +                while (i--)
>> +                        init_funcs[i].exit();
>> +
>> +        return err;
>>  }
>>  
>>  static void __exit xe_exit(void)
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center