Re: [PATCH v6 17/22] x86/resctrl: Assign/unassign counters by default when ABMC is enabled

[Moger, Babu]

Hi Reinette,

On 8/16/24 16:42, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/6/24 3:00 PM, Babu Moger wrote:
>> Assign/unassign counters on resctrl group creation/deletion. Two counters
>> are required per group, one for total event and one for local event.
>>
>> There are only limited number of counters for assignment. If the counters
>> are exhausted, report the warnings and continue. It is not required to
> 
> Regarding "report the warnings and continue", which warnings are you
> referring to?

I was referring to "rdt_last_cmd_puts("Out of ABMC counters\n");"

I will make that clear here.

> 
>> fail group creation for assignment failures. Users have the option to
>> modify the assignments later.
>>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>> v6: Removed the redundant comments on all the calls of
>>      rdtgroup_assign_cntrs. Updated the commit message.
>>      Dropped printing error message on every call of rdtgroup_assign_cntrs.
>>
>> v5: Removed the code to enable/disable ABMC during the mount.
>>      That will be another patch.
>>      Added arch callers to get the arch specific data.
>>      Renamed fuctions to match the other abmc function.
>>      Added code comments for assignment failures.
>>
>> v4: Few name changes based on the upstream discussion.
>>      Commit message update.
>>
>> v3: This is a new patch. Patch addresses the upstream comment to enable
>>      ABMC feature by default if the feature is available.
>> ---
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 55 ++++++++++++++++++++++++++
>>   1 file changed, 55 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 0c2215dbd497..d93c1d784b91 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2908,6 +2908,46 @@ static void schemata_list_destroy(void)
>>       }
>>   }
>>   +/*
>> + * Called when new group is created. Assign the counters if ABMC is
> 
> Please replace ABMC with resctrl fs generic terms.

Sure.

> 
>> + * already enabled. Two counters are required per group, one for total
>> + * event and one for local event. With limited number of counters,
>> + * the assignments can fail in some cases. But, it is not required to
>> + * fail the group creation. Users have the option to modify the
>> + * assignments after the group creation.
>> + */
>> +static int rdtgroup_assign_cntrs(struct rdtgroup *rdtgrp)
>> +{
>> +    int ret = 0;
>> +
>> +    if (!resctrl_arch_get_abmc_enabled())
>> +        return 0;
>> +
>> +    if (is_mbm_total_enabled())
>> +        ret = rdtgroup_assign_cntr(rdtgrp, QOS_L3_MBM_TOTAL_EVENT_ID);
>> +
>> +    if (!ret && is_mbm_local_enabled())
>> +        ret = rdtgroup_assign_cntr(rdtgrp, QOS_L3_MBM_LOCAL_EVENT_ID);
>> +
>> +    return ret;
>> +}
>> +
> 
> Reinette
> 

-- 
Thanks
Babu Moger