Re: [PATCH v6 02/24] x86/resctrl: kfree() rmid_ptrs from rdtgroup_exit()

[James Morse]

Hi Reinette,

On 02/10/2023 18:00, Reinette Chatre wrote:
> On 9/14/2023 10:21 AM, James Morse wrote:
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 725344048f85..a2158c266e41 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -3867,6 +3867,11 @@ int __init rdtgroup_init(void)
>>  
>>  void __exit rdtgroup_exit(void)
>>  {
>> +	struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>> +
>> +	if (r->mon_capable)
>> +		resctrl_exit_mon_l3_config(r);
>> +
>>  	debugfs_remove_recursive(debugfs_resctrl);
>>  	unregister_filesystem(&rdt_fs_type);
>>  	sysfs_remove_mount_point(fs_kobj, "resctrl");
> 
> You did not respond to me when I requested that this be done differently [1].
> Without a response letting me know the faults of my proposal or following the
> recommendation I conclude that my feedback was ignored. 

Not so - I just trimmed the bits that didn't need a response. I can respond 'Yes' to each
one if you prefer, but I find that adds more noise than signal.

This is my attempt at 'doing the cleanup properly', which is what you said your preference
was. (no machine on the planet can ever run this code, the __exit section is always
discarded by the linker).

Reading through again, I missed that you wanted this called from resctrl_exit(). (The
naming suggests I did this originally, but it didn't work out).
I don't think this works as the code in resctrl_exit() remains part of the arch code after
the move, but allocating rmid_ptrs[] stays part of the fs code.

resctrl_exit() in core.c gets renamed as resctrl_arch_exit(), and rdtgroup_exit() takes on
the name resctrl_exit() as its part of the exposed interface.


Thanks,

James