Re: [PATCH v1 12/31] x86/resctrl: Move max_{name,data}_width into resctrl code

[Reinette Chatre]

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