Received-SPF: pass (google.com: domain of linux-kernel+bounces-166809-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <4875492c-13cd-4a77-a42f-243fe0f868a2@intel.com>
Date: Thu, 2 May 2024 10:52:15 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [RFC PATCH v3 17/17] x86/resctrl: Introduce interface to modify
 assignment states of the groups
To: Dave Martin <Dave.Martin@arm.com>, Babu Moger <babu.moger@amd.com>
CC: <corbet@lwn.net>, <fenghua.yu@intel.com>, <tglx@linutronix.de>,
	<mingo@redhat.com>, <bp@alien8.de>, <dave.hansen@linux.intel.com>,
	<x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>,
	<rdunlap@infradead.org>, <tj@kernel.org>, <peterz@infradead.org>,
	<yanjiewtw@gmail.com>, <kim.phillips@amd.com>, <lukas.bulwahn@gmail.com>,
	<seanjc@google.com>, <jmattson@google.com>, <leitao@debian.org>,
	<jpoimboe@kernel.org>, <rick.p.edgecombe@intel.com>,
	<kirill.shutemov@linux.intel.com>, <jithu.joseph@intel.com>,
	<kai.huang@intel.com>, <kan.liang@linux.intel.com>,
	<daniel.sneddon@linux.intel.com>, <pbonzini@redhat.com>,
	<sandipan.das@amd.com>, <ilpo.jarvinen@linux.intel.com>,
	<peternewman@google.com>, <maciej.wieczor-retman@intel.com>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<eranian@google.com>, <james.morse@arm.com>
References: <cover.1711674410.git.babu.moger@amd.com>
 <f7dac996d87b4144e4c786178a7fd3d218eaebe8.1711674410.git.babu.moger@amd.com>
 <ZjO9hpuLz/jJYqvT@e133380.arm.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ZjO9hpuLz/jJYqvT@e133380.arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WlJLd0IwUlNFam5QdTN3QVpMbWpzUlFIOUY4M1k4Qjd2ekphK2xiWWdjMDlQ?=
 =?utf-8?B?eS9xYnFaYjhES3NuWXd2OTVLQ0FENUkrM3F5YXpLLzhRUEozZGNMOGNTUjd3?=
 =?utf-8?B?LysvRUJXOFRydXJFQkloOC9scDFPYThyZTlURitMQ0ZSREQ3dEJmTktTYTZv?=
 =?utf-8?B?QUZVQkc5dTh5V05LYkdBUHF1MEZzWFJzbC9ydHkzWTdYZ3h2UmVCbDAxaGF5?=
 =?utf-8?B?Z0RCZ1NFeFFOZW9zeFF2MWZNbHpZNDVWeGl1SllLVlJhU1VEckNaVnZKK1Az?=
 =?utf-8?B?WEJ4WWY1N2dISWpiZEpNcExLSUR0Vnh5U0RWWmw5dzVKMHVhVWd6YWlidWdj?=
 =?utf-8?B?U1doWnJhVWtBQ3hrM3UzT0k2V0FnMlpFWTZ4UWRqeXF5TnhIa0NaWG93a2ll?=
 =?utf-8?B?M1ZDb1cvR1JPMitveG9kY0V0TzBnamx5a1JLbyt1Wi9pZkxzRzFaUTljUTBn?=
 =?utf-8?B?dXJmWEVMSXVjZi9TNTlGQjBJMGFNVDBVZUZrWlBLTVU1aENhbjgxbkZkc0d3?=
 =?utf-8?B?N2VnTzBFbyt0ZFIxUGUyNzdBNzY0SHVicHZuYW01ZlZ5MFc1UXM2QU85QnM3?=
 =?utf-8?B?VHFSbk13cUdMcEJIZDc1eng1UmNVSjZHSG9wZ1Q3TVBuK0tRVFRhN3NxYjJt?=
 =?utf-8?B?bktCNERYTnA4R1UxUUxkTHpwM1p0REZJWWdSWnZTSmRWT1dQdGZZZnpIUVEy?=
 =?utf-8?B?NTZZRWxnek83aFhrUFlLSnNpa0JQWU1ndEZjalluK3B5UllTS1FrTkJwR1F0?=
 =?utf-8?B?dXduWWRiSmdTRlBheU5ZeEttNXp0ei9ORjYyalJUVFhRRitocU1DQVA2KzVk?=
 =?utf-8?B?ZkY0cTVZZjdEelNqRkVBTEs2RHV4bERSUjRLbytnVFRnRm9YYzZjNndreEp2?=
 =?utf-8?B?MXFqKzJwaWZXQVlCQzljSHVpbW9iWU1wNTRBblVTeEE2eTVFSHY2dDlFRllR?=
 =?utf-8?B?cUkwZ0ZhTzd2OVJtajdXT0dPRTlHY1NNUUNYMS9XR2RybCtNb2JYNzAxRzRh?=
 =?utf-8?B?N0xqK1dLZGIrQ3ZGM2l1eHhpd0tzMERBb2t5L0Y4MEFWQWFSa2VxTHViVFFJ?=
 =?utf-8?B?b0VZTVJ3dkIxSExnV0lJWm5rMGthRjJaVG5tWlR3TmFrL21PK2pjRVlJcmgr?=
 =?utf-8?B?empqTlhxU3hBUnB1bUF5Z0VrMjJLN0xOcXVGWVNJMm56ZzgwcjdTblM1eG9t?=
 =?utf-8?B?OGpaR01ZbjZORVRlREtIWXFrOWJoZ3FLRThiLzloazRhV1BxNXlINFhqeFNM?=
 =?utf-8?B?Qk5vbEsrRG5tNmVDU09oemcwSnk4R2lBeUtrbEVtR0F4akhhWXZUTmxrNDEy?=
 =?utf-8?B?czBvUEwvOXFzY3U2QnA1MU9yM2E1UEgzMFB2RTVKTkI1a1Y5bTVldHRvSi91?=
 =?utf-8?B?cDdaM0FiRE8wVEd3NkE3elpTQ3FGakp2QjhNTlhIMHplWGNqc2h0K3M5cEtB?=
 =?utf-8?B?QXJpYVplRDN0QzdWeVFxTC9nVzArS2FkMzZOSkxJL2JsMmNURGVzNCtjVm5S?=
 =?utf-8?B?RkdVTlptYkhaVkk5ckw1enpjNXk1bll3RjkvdFpHMEp4aDNqK0g3UDBpSnFB?=
 =?utf-8?B?Q3ZtU0lTSkwrMndxMlcrUDBydXAxckEyTFk0WFNXNzI4SjVmMU8va2lQaTdD?=
 =?utf-8?B?TFRucE5vTVYvMGRiNHVTNmZKY2NMbElqK3ZyRlljZVIvSlQxSi9LUEVKM1I1?=
 =?utf-8?B?bHVtS0JkZHJJVVhKd1d4WGVYOUNZZ2t2M2hmWCtWWnRZQjNLVWlxVG11WDh5?=
 =?utf-8?B?TkRFZ0g0d2U1OFl2dE41dVhmQWZDOWJpSEFLRkozRmZ6RFlsK2hPYk02cHpP?=
 =?utf-8?B?b2ErS3JBR0p4eGdjS0hGZG1BaW9hKzNxMzFtZGU2R3dzZ21sM1ZlWVpPd1Rv?=
 =?utf-8?B?SXNlaXQvM0dYNU1HM2owcDNqZFc1NjB3a3dTbUN2cmNoYmF4N1diRTg3a1pK?=
 =?utf-8?B?a3pEeG5CLzIzM052Uktyc25iSEJ6a1FFUGRmNkMrMGdEVUQ4bkxBQmdBaWgw?=
 =?utf-8?B?WWtldDY4QkozcS81bnZVTHNnS0orWG5VS1pLUitoV2tHNkV3dGZYZjk0QWt6?=
 =?utf-8?B?Q3ZoOVdmeW5BVXRFRU5HRjhoMS9zaGhtVFk4azhpbzNweis1VFBNb0poelY5?=
 =?utf-8?B?cUFVMjZOZUNHb1FIMFkzODhPcDRqN29nYVZISHlFOHprOVp1VEJQaG5RbVpU?=
 =?utf-8?B?TEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 001af304-0e5d-4185-1a4b-08dc6ad09cd7
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2024 17:52:19.8141
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dgr1Z1iMJvVKwtoijXccDeVNz+P4DJePYzHBhwHVuuxm9yg32JC54kbOQ9bVcyjWJOqx115XoXNA9fr52TjpomqWtVc1ns8iOwagj3nUFnU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7133
X-OriginatorOrg: intel.com

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@e133380.arm.com/
https://lore.kernel.org/lkml/ZiervIprcwoApAqw@e133380.arm.com/

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

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

> 
> 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