max_name_width and max_data_width are used to pad the strings in the
resctrl schemata file.

This should be part of the fs code as it influences the user-space
interface, but currently max_data_width is generated by the arch init code.
max_name_width is already managed by schemata_list_add().

Move the variables and max_data_width's initialisation code to rdtgroup.c.
There is no need for an extra rdt_init_padding() helper as the length
of the name can be considered when schemata_list_add() creates each
schema entry.

Signed-off-by: James Morse <james.morse@arm.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     | 22 ----------------------
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 12 ++++++++++++
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 9a09a64bbb7f..9551ca4a6480 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -44,12 +44,6 @@ static DEFINE_MUTEX(domain_list_lock);
  */
 DEFINE_PER_CPU(struct resctrl_pqr_state, pqr_state);
 
-/*
- * Used to store the max resource name width and max resource data width
- * to display the schemata in a tabular format
- */
-int max_name_width, max_data_width;
-
 /*
  * Global boolean for rdt_alloc which is true if any
  * resource allocation is enabled.
@@ -648,20 +642,6 @@ static int resctrl_arch_offline_cpu(unsigned int cpu)
 	return 0;
 }
 
-/*
- * Choose a width for the resource name and resource data based on the
- * resource that has widest name and cbm.
- */
-static __init void rdt_init_padding(void)
-{
-	struct rdt_resource *r;
-
-	for_each_alloc_capable_rdt_resource(r) {
-		if (r->data_width > max_data_width)
-			max_data_width = r->data_width;
-	}
-}
-
 enum {
 	RDT_FLAG_CMT,
 	RDT_FLAG_MBM_TOTAL,
@@ -959,8 +939,6 @@ static int __init resctrl_arch_late_init(void)
 	if (!get_rdt_resources())
 		return -ENODEV;
 
-	rdt_init_padding();
-
 	state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
 				  "x86/resctrl/cat:online:",
 				  resctrl_arch_online_cpu,
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 6cf4ebe9c058..e736e4d20f63 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -58,6 +58,12 @@ static struct kernfs_node *kn_mongrp;
 /* Kernel fs node for "mon_data" directory under root */
 static struct kernfs_node *kn_mondata;
 
+/*
+ * Used to store the max resource name width and max resource data width
+ * to display the schemata in a tabular format
+ */
+int max_name_width, max_data_width;
+
 static struct seq_buf last_cmd_status;
 static char last_cmd_status_buf[512];
 
@@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
 	if (cl > max_name_width)
 		max_name_width = cl;
 
+	/*
+	 * Choose a width for the resource data based on the resource that has
+	 * widest name and cbm.
+	 */
+	max_data_width = max(max_data_width, r->data_width);
+
 	INIT_LIST_HEAD(&s->list);
 	list_add(&s->list, &resctrl_schema_all);
 
-- 
2.39.2

Hi James,

On 3/21/2024 9:50 AM, James Morse wrote:
> @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
>  	if (cl > max_name_width)
>  		max_name_width = cl;
>  
> +	/*
> +	 * Choose a width for the resource data based on the resource that has
> +	 * widest name and cbm.

Please check series to ensure upper case is used for acronyms.

> +	 */
> +	max_data_width = max(max_data_width, r->data_width);
> +
>  	INIT_LIST_HEAD(&s->list);
>  	list_add(&s->list, &resctrl_schema_all);
>  

Reinette

On Mon, Apr 08, 2024 at 08:19:15PM -0700, Reinette Chatre wrote:
> Hi James,
> 
> On 3/21/2024 9:50 AM, James Morse wrote:
> > @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
> >  	if (cl > max_name_width)
> >  		max_name_width = cl;
> >  
> > +	/*
> > +	 * Choose a width for the resource data based on the resource that has
> > +	 * widest name and cbm.
> 
> Please check series to ensure upper case is used for acronyms.

[...]

> Reinette

This patch is just moving existing code around AFAICT.  See:
commit de016df88f23 ("x86/intel_rdt: Update schemata read to show data in tabular format")

Since no new usage of any term is being introduced here, can it be
left as-is?

There seem to be other uses of "cbm" with this sense in the resctrl
code already.

Cheers
---Dave

Hi Dave,

On 4/11/2024 7:15 AM, Dave Martin wrote:
> On Mon, Apr 08, 2024 at 08:19:15PM -0700, Reinette Chatre wrote:
>> Hi James,
>>
>> On 3/21/2024 9:50 AM, James Morse wrote:
>>> @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
>>>  	if (cl > max_name_width)
>>>  		max_name_width = cl;
>>>  
>>> +	/*
>>> +	 * Choose a width for the resource data based on the resource that has
>>> +	 * widest name and cbm.
>>
>> Please check series to ensure upper case is used for acronyms.
> 
> [...]
> 
>> Reinette
> 
> This patch is just moving existing code around AFAICT.  See:
> commit de016df88f23 ("x86/intel_rdt: Update schemata read to show data in tabular format")
> 
> Since no new usage of any term is being introduced here, can it be
> left as-is?
> 
> There seem to be other uses of "cbm" with this sense in the resctrl
> code already.

I am not asking to change all existing usages of these terms but in
any new changes, please use upper case for acronyms.

Reinette

Hi Reinette,

On Thu, Apr 11, 2024 at 10:38:20AM -0700, Reinette Chatre wrote:
> Hi Dave,
> 
> On 4/11/2024 7:15 AM, Dave Martin wrote:
> > On Mon, Apr 08, 2024 at 08:19:15PM -0700, Reinette Chatre wrote:
> >> Hi James,
> >>
> >> On 3/21/2024 9:50 AM, James Morse wrote:
> >>> @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
> >>>  	if (cl > max_name_width)
> >>>  		max_name_width = cl;
> >>>  
> >>> +	/*
> >>> +	 * Choose a width for the resource data based on the resource that has
> >>> +	 * widest name and cbm.
> >>
> >> Please check series to ensure upper case is used for acronyms.
> > 
> > [...]
> > 
> >> Reinette
> > 
> > This patch is just moving existing code around AFAICT.  See:
> > commit de016df88f23 ("x86/intel_rdt: Update schemata read to show data in tabular format")
> > 
> > Since no new usage of any term is being introduced here, can it be
> > left as-is?
> > 
> > There seem to be other uses of "cbm" with this sense in the resctrl
> > code already.
> 
> I am not asking to change all existing usages of these terms but in
> any new changes, please use upper case for acronyms.

While there is a general argument to be made here, it sounds from this
like you are not requesting a change to this patch; can you confirm?

Cheers
---Dave

Hi Dave,

On 4/17/2024 7:41 AM, Dave Martin wrote:
> On Thu, Apr 11, 2024 at 10:38:20AM -0700, Reinette Chatre wrote:
>> On 4/11/2024 7:15 AM, Dave Martin wrote:
>>> On Mon, Apr 08, 2024 at 08:19:15PM -0700, Reinette Chatre wrote:
>>>> Hi James,
>>>>
>>>> On 3/21/2024 9:50 AM, James Morse wrote:
>>>>> @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
>>>>>  	if (cl > max_name_width)
>>>>>  		max_name_width = cl;
>>>>>  
>>>>> +	/*
>>>>> +	 * Choose a width for the resource data based on the resource that has
>>>>> +	 * widest name and cbm.
>>>>
>>>> Please check series to ensure upper case is used for acronyms.
>>>
>>> [...]
>>>
>>>> Reinette
>>>
>>> This patch is just moving existing code around AFAICT.  See:
>>> commit de016df88f23 ("x86/intel_rdt: Update schemata read to show data in tabular format")
>>>
>>> Since no new usage of any term is being introduced here, can it be
>>> left as-is?
>>>
>>> There seem to be other uses of "cbm" with this sense in the resctrl
>>> code already.
>>
>> I am not asking to change all existing usages of these terms but in
>> any new changes, please use upper case for acronyms.
> 
> While there is a general argument to be made here, it sounds from this
> like you are not requesting a change to this patch; can you confirm?

Sorry for confusion. I do think in a small change like this it is a good
opportunity to make sure the style is clean. Since this changes the code
anyway, it might as well ensure the style is clean. So, yes, could
you please use CBM instead of cbm.

The final patch of this series is in a different category altogether and I
do not think style changes will be appropriate in it.

Reinette

On Wed, Apr 17, 2024 at 10:16:45PM -0700, Reinette Chatre wrote:
> Hi Dave,
> 
> On 4/17/2024 7:41 AM, Dave Martin wrote:
> > On Thu, Apr 11, 2024 at 10:38:20AM -0700, Reinette Chatre wrote:
> >> On 4/11/2024 7:15 AM, Dave Martin wrote:
> >>> On Mon, Apr 08, 2024 at 08:19:15PM -0700, Reinette Chatre wrote:
> >>>> Hi James,
> >>>>
> >>>> On 3/21/2024 9:50 AM, James Morse wrote:
> >>>>> @@ -2595,6 +2601,12 @@ static int schemata_list_add(struct rdt_resource *r, enum resctrl_conf_type type
> >>>>>  	if (cl > max_name_width)
> >>>>>  		max_name_width = cl;
> >>>>>  
> >>>>> +	/*
> >>>>> +	 * Choose a width for the resource data based on the resource that has
> >>>>> +	 * widest name and cbm.
> >>>>
> >>>> Please check series to ensure upper case is used for acronyms.
> >>>
> >>> [...]
> >>>
> >>>> Reinette
> >>>
> >>> This patch is just moving existing code around AFAICT.  See:
> >>> commit de016df88f23 ("x86/intel_rdt: Update schemata read to show data in tabular format")
> >>>
> >>> Since no new usage of any term is being introduced here, can it be
> >>> left as-is?
> >>>
> >>> There seem to be other uses of "cbm" with this sense in the resctrl
> >>> code already.
> >>
> >> I am not asking to change all existing usages of these terms but in
> >> any new changes, please use upper case for acronyms.
> > 
> > While there is a general argument to be made here, it sounds from this
> > like you are not requesting a change to this patch; can you confirm?
> 
> Sorry for confusion. I do think in a small change like this it is a good
> opportunity to make sure the style is clean. Since this changes the code
> anyway, it might as well ensure the style is clean. So, yes, could
> you please use CBM instead of cbm.

OK; I had thought that we might be introducing a new inconsistency here
by making such a change, but looking at upstream, "CBM" is prevalent in
comments in the preexisting x86 code.  I should have checked that before;
apologies for the unnecessary back-and-forth on this.

So, sure, it makes sense to change it.

Noted.

Cheers
---Dave