Re: [RFC PATCH v3 17/17] x86/resctrl: Introduce interface to modify assignment states of the groups

From: Reinette Chatre
Date: Thu May 02 2024 - 13:52:37 EST


Hi Dave,

On 5/2/2024 9:21 AM, Dave Martin wrote:
> On Thu, Mar 28, 2024 at 08:06:50PM -0500, Babu Moger wrote:
>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>> index 2d96565501ab..64ec70637c66 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -328,6 +328,77 @@ with the following files:
>> None of events are assigned on this mon group. This is a child
>> monitor group of the non default control mon group.
>>
>> + Assignment state can be updated by writing to this interface.
>> +
>> + NOTE: Assignment on one domain applied on all the domains. User can
>> + pass one valid domain and assignment will be updated on all the
>> + available domains.
>> +
>> + Format is similar to the list format with addition of op-code for the
>> + assignment operation.
>> +
>> + * Default CTRL_MON group:
>> + "//<domain_id><op-code><assignment_flags>"
>> +
>> + * Non-default CTRL_MON group:
>> + "<CTRL_MON group>//<domain_id><op-code><assignment_flags>"
>> +
>> + * Child MON group of default CTRL_MON group:
>> + "/<MON group>/<domain_id><op-code><assignment_flags>"
>> +
>> + * Child MON group of non-default CTRL_MON group:
>> + "<CTRL_MON group>/<MON group>/<domain_id><op-code><assignment_flags>"
>
> The final bullet seems to cover everything, if we allow <CTRL_MON group>
> and <MON group> to be independently empty strings to indicate the
> default control and/or monitoring group respectively.
>
> Would that be simpler than treating this as four separate cases?
>
> Also, will this go wrong if someone creates a resctrl group with '\n'
> (i.e., a newline character) in the name?

There is a check for this in rdtgroup_mkdir().

>
>> +
>> + Op-code can be one of the following:
>> + ::
>> +
>> + = Update the assignment to match the flags
>> + + Assign a new state
>> + - Unassign a new state
>> + _ Unassign all the states
>
> I can't remember whether I already asked this, but is "_" really
> needed here?

Asked twice:
https://lore.kernel.org/lkml/ZiaRXrmDDjc194JI@xxxxxxxxxxxxxxx/
https://lore.kernel.org/lkml/ZiervIprcwoApAqw@xxxxxxxxxxxxxxx/

Answered:
https://lore.kernel.org/lkml/4cd220cc-2e8e-4193-b01a-d3cd798c7118@xxxxxxx/

You seemed ok with answer then:
https://lore.kernel.org/lkml/ZiffF93HM8bE3qo7@xxxxxxxxxxxxxxx/

>
> Wouldn't it be the case that
>
> //*_
>
> would mean just the same thing as
>
> //*=_
>
> ...? (assuming the "*" = "all domains" convention already discussed)
>
> Maybe I'm missing something here.

I believe have an explicit operator ("+", "=", or "-") simplifies
parsing while providing an interface consistent with what users are already
used to.

Reinette