Re: [PATCH v8 09/25] x86/resctrl: Add __init attribute to dom_data_init()

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 10/16/24 10:32 AM, Moger, Babu wrote:
> 
> 
> On 10/15/24 22:13, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 10/9/24 10:39 AM, Babu Moger wrote:
>>> dom_data_init() is only called during the __init sequence.
>>> Add __init attribute like the rest of call sequence.
>>>
>>> While at it, pass 'struct rdt_resource' to dom_data_init() and
>>> dom_data_exit() which will be used for mbm counter __init and__exit
>>> call sequence.
>>
>> This patch needs to be split. Please move fixes to beginning of series and
>> move the addition of the parameter to the patch where it is first used/needed.
> 
> Sure. Will move the fixes to the beginning.
> 
>>
>>>
>>> Fixes: bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")
>>
>> For this change I think the following Fixes tag would be more accurate:
>> Fixes: 6a445edce657 ("x86/intel_rdt/cqm: Add RDT monitoring initialization")
>>
>> I think for a complete fix of the above commit it also needs to add __init
>> storage class to l3_mon_evt_init().
> 
> Yes. Sure.
> 
>>
>> The __init storage class is also missing from rdt_get_mon_l3_config() ...
> 
> 1 internal.h _int rdt_get_mon_l3_config(struct rdt_resource *r);
> 2 monitor.c  int __init rdt_get_mon_l3_config(struct rdt_resource *r)
> 
> rdt_get_mon_l3_config() has __init attribute already. But prototype in
> internal.h does not add the '__init'. Looks like that is ok.

I also think it may technically be ok since as far as I understand attributes
the attributes will be merged. Even so, doing so does not match the current
style where the storage class of declaration and definition are the same. See
for example the partner function rdt_put_mon_l3_config().

> 
> 
>> fixing that would indeed need the Fixes tag below:
>> Fixes: bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()"
> 
> How about addressing both dom_data_init() and l3_mon_evt_init() in a
> single patch and adding 2 fixes flags?

... and add __init to declaration of rdt_get_mon_l3_config() ?

> 
> Fixes: 6a445edce657 ("x86/intel_rdt/cqm: Add RDT monitoring initialization")
> Fixes: bd334c86b5d7 ("x86/resctrl: Add __init attribute to
> rdt_get_mon_l3_config()")

Reinette