[RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init

Babu Moger posted 17 patches 1 year, 10 months ago
There is a newer version of this series
[RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init
Posted by Babu Moger 1 year, 10 months ago
Consolidate multiple fflags initialization into one function.

Remove thread_throttle_mode_init, mbm_config_rftype_init and
consolidate them into resctrl_file_fflags_init.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v3: No changes.

v2: New patch. New function to consolidate fflags initialization
---
 arch/x86/kernel/cpu/resctrl/core.c     |  4 +++-
 arch/x86/kernel/cpu/resctrl/internal.h |  4 ++--
 arch/x86/kernel/cpu/resctrl/monitor.c  |  6 ++++--
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 +++-------------
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index bb82b392cf5d..50e9ec5e547b 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -229,7 +229,9 @@ static bool __get_mem_config_intel(struct rdt_resource *r)
 		r->membw.throttle_mode = THREAD_THROTTLE_PER_THREAD;
 	else
 		r->membw.throttle_mode = THREAD_THROTTLE_MAX;
-	thread_throttle_mode_init();
+
+	resctrl_file_fflags_init("thread_throttle_mode",
+				 RFTYPE_CTRL_INFO | RFTYPE_RES_MB);
 
 	r->alloc_capable = true;
 
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index c4ae6f3993aa..722388621403 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -602,8 +602,8 @@ void cqm_handle_limbo(struct work_struct *work);
 bool has_busy_rmid(struct rdt_domain *d);
 void __check_limbo(struct rdt_domain *d, bool force_free);
 void rdt_domain_reconfigure_cdp(struct rdt_resource *r);
-void __init thread_throttle_mode_init(void);
-void __init mbm_config_rftype_init(const char *config);
+void __init resctrl_file_fflags_init(const char *config,
+				     unsigned long fflags);
 void rdt_staged_configs_clear(void);
 bool closid_allocated(unsigned int closid);
 int resctrl_find_cleanest_closid(void);
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index e5938bf53d5a..735b449039c1 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -1049,11 +1049,13 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 
 		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
 			mbm_total_event.configurable = true;
-			mbm_config_rftype_init("mbm_total_bytes_config");
+			resctrl_file_fflags_init("mbm_total_bytes_config",
+						 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
 		}
 		if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
 			mbm_local_event.configurable = true;
-			mbm_config_rftype_init("mbm_local_bytes_config");
+			resctrl_file_fflags_init("mbm_local_bytes_config",
+						 RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
 		}
 
 		if (resctrl_arch_has_abmc(r))
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 011e17efb1a6..dda71fb6c10e 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2022,24 +2022,14 @@ static struct rftype *rdtgroup_get_rftype_by_name(const char *name)
 	return NULL;
 }
 
-void __init thread_throttle_mode_init(void)
-{
-	struct rftype *rft;
-
-	rft = rdtgroup_get_rftype_by_name("thread_throttle_mode");
-	if (!rft)
-		return;
-
-	rft->fflags = RFTYPE_CTRL_INFO | RFTYPE_RES_MB;
-}
-
-void __init mbm_config_rftype_init(const char *config)
+void __init resctrl_file_fflags_init(const char *config,
+				     unsigned long fflags)
 {
 	struct rftype *rft;
 
 	rft = rdtgroup_get_rftype_by_name(config);
 	if (rft)
-		rft->fflags = RFTYPE_MON_INFO | RFTYPE_RES_CACHE;
+		rft->fflags = fflags;
 }
 
 /**
-- 
2.34.1
Re: [RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init
Posted by Reinette Chatre 1 year, 9 months ago
Hi Babu,

In shortlog, please use () to indicate function:
resctrl_file_fflags_init().

On 3/28/2024 6:06 PM, Babu Moger wrote:
> Consolidate multiple fflags initialization into one function.
> 
> Remove thread_throttle_mode_init, mbm_config_rftype_init and
> consolidate them into resctrl_file_fflags_init.

This changelog has no context and only describes what the code does,
which can be learned from reading the patch. Could you please
update changelog with context and motivation for this change?

Reinette
Re: [RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init
Posted by Moger, Babu 1 year, 9 months ago
Hi Reinette,

On 5/3/24 18:26, Reinette Chatre wrote:
> Hi Babu,
> 
> In shortlog, please use () to indicate function:
> resctrl_file_fflags_init().
> 
> On 3/28/2024 6:06 PM, Babu Moger wrote:
>> Consolidate multiple fflags initialization into one function.
>>
>> Remove thread_throttle_mode_init, mbm_config_rftype_init and
>> consolidate them into resctrl_file_fflags_init.
> 
> This changelog has no context and only describes what the code does,
> which can be learned from reading the patch. Could you please
> update changelog with context and motivation for this change?
How about this?

Consolidate multiple fflags initialization into one function.

The functions thread_throttle_mode_init() and mbm_config_rftype_init()
both initialize fflags for resctrl files. This can be simplified into one
function passing the file name and flags to be initialized. It also helps
code simplification of new flags initialized for ABMC feature.
-- 
Thanks
Babu Moger
Re: [RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init
Posted by Reinette Chatre 1 year, 9 months ago
Hi Babu,

On 5/6/2024 1:23 PM, Moger, Babu wrote:
> On 5/3/24 18:26, Reinette Chatre wrote:
>> Hi Babu,
>>
>> In shortlog, please use () to indicate function:
>> resctrl_file_fflags_init().
>>
>> On 3/28/2024 6:06 PM, Babu Moger wrote:
>>> Consolidate multiple fflags initialization into one function.
>>>
>>> Remove thread_throttle_mode_init, mbm_config_rftype_init and
>>> consolidate them into resctrl_file_fflags_init.
>>
>> This changelog has no context and only describes what the code does,
>> which can be learned from reading the patch. Could you please
>> update changelog with context and motivation for this change?
> How about this?
> 
> Consolidate multiple fflags initialization into one function.
> 
> The functions thread_throttle_mode_init() and mbm_config_rftype_init()
> both initialize fflags for resctrl files. This can be simplified into one
> function passing the file name and flags to be initialized. It also helps
> code simplification of new flags initialized for ABMC feature.

I am not sure what you intend here. Is this the entire intended changelog?
I expected it to reflect x86 requirements as per
Documentation/process/maintainer-tip.rst. Under "Changelog":

"It's also useful to structure the changelog into several paragraphs and not
lump everything together into a single one. A good structure is to explain
the context, the problem and the solution in separate paragraphs and this
order."

Reinette
Re: [RFC PATCH v3 04/17] x86/resctrl: Introduce resctrl_file_fflags_init
Posted by Moger, Babu 1 year, 9 months ago
Hi Reinette,

On 5/7/2024 3:27 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/6/2024 1:23 PM, Moger, Babu wrote:
>> On 5/3/24 18:26, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> In shortlog, please use () to indicate function:
>>> resctrl_file_fflags_init().
>>>
>>> On 3/28/2024 6:06 PM, Babu Moger wrote:
>>>> Consolidate multiple fflags initialization into one function.
>>>>
>>>> Remove thread_throttle_mode_init, mbm_config_rftype_init and
>>>> consolidate them into resctrl_file_fflags_init.
>>>
>>> This changelog has no context and only describes what the code does,
>>> which can be learned from reading the patch. Could you please
>>> update changelog with context and motivation for this change?
>> How about this?
>>
>> Consolidate multiple fflags initialization into one function.
>>
>> The functions thread_throttle_mode_init() and mbm_config_rftype_init()
>> both initialize fflags for resctrl files. This can be simplified into one
>> function passing the file name and flags to be initialized. It also helps
>> code simplification of new flags initialized for ABMC feature.
> 
> I am not sure what you intend here. Is this the entire intended changelog?
> I expected it to reflect x86 requirements as per
> Documentation/process/maintainer-tip.rst. Under "Changelog":
> 
> "It's also useful to structure the changelog into several paragraphs and not
> lump everything together into a single one. A good structure is to explain
> the context, the problem and the solution in separate paragraphs and this
> order."

I will try to elaborate this in next version.
Thanks
- Babu Moger