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.

Fixes: bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")
Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v8: New patch.
---
 arch/x86/kernel/cpu/resctrl/core.c     | 2 +-
 arch/x86/kernel/cpu/resctrl/internal.h | 2 +-
 arch/x86/kernel/cpu/resctrl/monitor.c  | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 49d147e2e4e5..00ad00258df2 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -1140,7 +1140,7 @@ static void __exit resctrl_exit(void)
 	rdtgroup_exit();
 
 	if (r->mon_capable)
-		rdt_put_mon_l3_config();
+		rdt_put_mon_l3_config(r);
 }
 
 __exitcall(resctrl_exit);
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index a45ae410274c..92eae4672312 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -633,7 +633,7 @@ void closid_free(int closid);
 int alloc_rmid(u32 closid);
 void free_rmid(u32 closid, u32 rmid);
 int rdt_get_mon_l3_config(struct rdt_resource *r);
-void __exit rdt_put_mon_l3_config(void);
+void __exit rdt_put_mon_l3_config(struct rdt_resource *r);
 bool __init rdt_cpu_has(int flag);
 void mon_event_count(void *info);
 int rdtgroup_mondata_show(struct seq_file *m, void *arg);
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 7aa579a99501..66b06574f660 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -983,7 +983,7 @@ void mbm_setup_overflow_handler(struct rdt_mon_domain *dom, unsigned long delay_
 		schedule_delayed_work_on(cpu, &dom->mbm_over, delay);
 }
 
-static int dom_data_init(struct rdt_resource *r)
+static __init int dom_data_init(struct rdt_resource *r)
 {
 	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
 	u32 num_closid = resctrl_arch_get_num_closid(r);
@@ -1044,7 +1044,7 @@ static int dom_data_init(struct rdt_resource *r)
 	return err;
 }
 
-static void __exit dom_data_exit(void)
+static void __exit dom_data_exit(struct rdt_resource *r)
 {
 	mutex_lock(&rdtgroup_mutex);
 
@@ -1247,9 +1247,9 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 	return 0;
 }
 
-void __exit rdt_put_mon_l3_config(void)
+void __exit rdt_put_mon_l3_config(struct rdt_resource *r)
 {
-	dom_data_exit();
+	dom_data_exit(r);
 }
 
 void __init intel_rdt_mbm_apply_quirk(void)
-- 
2.34.1

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.

> 
> 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().

The __init storage class is also missing from rdt_get_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()"

Reinette

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.


> 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?

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

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

Hi Reinette,

On 10/16/24 13:55, Reinette Chatre wrote:
> 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().

Sure.

> 
>>
>>
>>> 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() ?

Sure. Will do.

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

-- 
Thanks
Babu Moger