Re: [PATCH v11 15/23] x86/resctrl: Add the functionality to assigm MBM events

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 2/7/25 1:10 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 2/5/2025 7:05 PM, Reinette Chatre wrote:
>> On 1/22/25 12:20 PM, Babu Moger wrote:
>>> The mbm_cntr_assign mode offers several counters that can be assigned
>>
>> This "several counters" contradicts the "very small number of assignable
>> counters" used in earlier patch to justify how counters are managed.
> 
> How about?
> 
> The mbm_cntr_assign mode offers "num_mbm_cntrs" number of counters that can be assigned to an RMID, event pair and monitor the bandwidth as long as it is assigned.

Sure. The word "several" can just be dropped from original also. The concern is not
the language but instead that the description moves from "several" in one patch
and then "limited" in the next patch.


...

>>> +        }
>>> +
>>> +        ret = resctrl_config_cntr(r, d, evtid, rdtgrp->mon.rmid, rdtgrp->closid,
>>> +                      cntr_id, true);
>>> +        if (ret) {
>>> +            rdt_last_cmd_printf("Assignment failed on domain %d\n", d->hdr.id);
>>
>> I assume this targets the scenario when user space requests "all" domains to be changed
>> and the error message in resctrl_process_flags() will then print "*" instead of the
>> actual domain ID. If this is the goal to give more detail to error then the event
>> can be displayed also?
> 
> Sure. Will change it to.
> 
> rdt_last_cmd_printf("Assignment of event %d failed on domain %d\n", d->hdr.id, evtid);

ok, printing the event ID should be OK since the ID will be part of resctrl fs and
not architecture specific. Please just swap last two parameters.

Reinette