Re: [PATCH v5 19/20] x86/resctrl: Introduce interface to list monitor states of all the groups

Reinette Chatre
2024-08-01T21:37:25Z

Hi Babu,

On 7/17/24 8:22 AM, Moger, Babu wrote:
> On 7/12/24 17:16, Reinette Chatre wrote:
>> On 7/3/24 2:48 PM, Babu Moger wrote:

>>> +     */
>>> +    if (rdtgrp->mon.cntr_id[0] != MON_CNTR_UNSET)
>>> +        if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[0],
>>> rdtgrp->mon.rmid))
>>> +            dom_state |= ASSIGN_TOTAL;
>>> +
>>> +    if (rdtgrp->mon.cntr_id[1] != MON_CNTR_UNSET)
>>> +        if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[1],
>>> rdtgrp->mon.rmid))
>>> +            dom_state |= ASSIGN_LOCAL;
>>> +
>>> +    switch (dom_state) {
>>> +    case ASSIGN_NONE:
>>> +        *tmp++ = '_';
>>> +        break;
>>> +    case (ASSIGN_TOTAL | ASSIGN_LOCAL):
>>> +        *tmp++ = 't';
>>> +        *tmp++ = 'l';
>>> +        break;
>>> +    case ASSIGN_TOTAL:
>>> +        *tmp++ = 't';
>>> +        break;
>>> +    case ASSIGN_LOCAL:
>>> +        *tmp++ = 'l';
>>> +        break;
>>> +    default:
>>> +        break;
>>> +    }
>>
>> This switch statement does not scale. Adding new flags will be painful.
>> Can flags not
>> just incrementally be printed as learned from hardware with "_" printed as
>> last resort?
>> This would elimininate need for these "ASSIGN" flags.
>
> Let me try to understand this.
>
> You want to remove switch statement.
>
> if (rdtgrp->mon.cntr_id[0] != MON_CNTR_UNSET)
> if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[0], rdtgrp->mon.rmid))
> *tmp++ = 't';
>
> if (rdtgrp->mon.cntr_id[1] != MON_CNTR_UNSET)
> if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[1], rdtgrp->mon.rmid))
> *tmp++ = 'l';
>
> If none of these flags are available, then
> *tmp++ = '_';
>
> Is that the idea?

Indeed. Thank you. Can this be done without hard coding the counter index?

Reinette