Received-SPF: pass (google.com: domain of linux-kernel+bounces-168232-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <98e06625-7e62-4614-969c-075462c2d27e@intel.com>
Date: Fri, 3 May 2024 14:15:45 -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>
CC: Babu Moger <babu.moger@amd.com>, <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>
 <4875492c-13cd-4a77-a42f-243fe0f868a2@intel.com>
 <ZjT6b/nbZWjm0vw+@e133380.arm.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ZjT6b/nbZWjm0vw+@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?MGxueU8xdUp5SGJFV3RGUndRaVhqMGlKcHZiL1FTc3lBYVduZzBCelFuMCti?=
 =?utf-8?B?QVZtazkwbW5NOGhDOHhkZllCS3czcDBRSWlUZGNjeFgydk1XZmhtbXpoc2NS?=
 =?utf-8?B?RXNaYmp1S2V6NzNyaWJEcFNaZVV0bVF1QkZsd0JwWFVzUzRFYlRnTlVjZG1o?=
 =?utf-8?B?T0NqYWNHVU0zMU5oK0llNVpCUTdoNTVHelUrMysyNnVvcExDZ2dmdXljWHF2?=
 =?utf-8?B?elNYTjdxZU85VkUwb1RqS1hiVXpZU2x3OUhISXNnd093eVRKekNIVHk5NVJq?=
 =?utf-8?B?ZU5PWGY1WkptUy82VGtGR1NVaUQycXNXdlFLU3FYV3ZDWFpXWVdwMGVrYnph?=
 =?utf-8?B?ZW8wQm53eTBod1JtSDIxL0NXdHd4WEp2b3hYWm9Vc1piQ25YYWN1OGF4R0dK?=
 =?utf-8?B?Z0l1eHVSNlVnOVBlR0Q3Q2k4MnNQZE1yenF3eXorOEVWRmFaSE5uNlh2L0dr?=
 =?utf-8?B?WlpWUjdFdnF1NENTQWU3cm5ad3p3Yk41SXhpMjVrWHhJdHFhaEtpTE1YcDVG?=
 =?utf-8?B?S3JQcVZVWVM2bVprRExsa3luZEZRbFpiZjFWMzdrT2UwWmNxWGhqQ2hUaEVO?=
 =?utf-8?B?SFU4MFIwN2E0M1hBSEVoKzhrSXFIRCtXaGhtSE1qV09lMTRsSE5rTmd4Yks5?=
 =?utf-8?B?K0FFci9LWnlJQVFlYW1tQy8xT05hSUFKTEdYQy9WUHYwNjFxVytOZVZ2VFpF?=
 =?utf-8?B?a3h6M1NMSi9FemhScGlJRGJYNmR0dElBM3N5VXpDWG1PcG9EamRWcTA0WHpK?=
 =?utf-8?B?UzRiU2MrYkJ4aGR1dFduYkJVcEJlUEE1WDhwREhPV08zV0VzUVkzT1p6S2Ux?=
 =?utf-8?B?VFlOcVVxOVlCazUzVXB4S3VIYjQyRkwvV2lpcWtHOVdwTHNmU3dHb0VJaW1H?=
 =?utf-8?B?WVFxMnJyeHFmb1l1ekdBSmhPVmRMcDdiRjk4MzE5QzByK0lRcGE4MFBDYVgz?=
 =?utf-8?B?RDE2Y2sraWNVVjN0VDFBT3RKUE1qWGZubzBnbkdUWC9XcHArSStWZEwvNkd4?=
 =?utf-8?B?VGpWdFQyRlYxVGpVMnRpajM0M2lxZWFMYXBRZ0JhamtpKy9lY0d5bVUrZHBy?=
 =?utf-8?B?WEh0clEwdjJJaTZOSUxmRC9RRC9xL05ObUxYbWtXL2VxQks1aUgzM3ZWZmpP?=
 =?utf-8?B?aitTZ093K1dPZkhsb1M4aUhqRTVQanBZSDVLNFB3SHVyK3ZSVmVBUVBUMnBr?=
 =?utf-8?B?cTNYeUg2anA4UjBXWDhOdzFvdERwN2F4bmsyeTNEMUx4VXpYUm5YcUMxaGpT?=
 =?utf-8?B?dGdFT1dnRDhxL1BZME5HSjAvYWwrZXJSNk55NlUySXVpbmpqYzI5ZVhkWVVo?=
 =?utf-8?B?YkovNVN0QWJuRjVnUWZmNGxSc1ZCZUVZdFpiOTIzbUpZMU9aSStyWFVscE5C?=
 =?utf-8?B?bDI4UG4xQS9VZUI3TUFtTWRiYWdzcHFJTnhFaUJVTjRXbkt1R0tRZjdMZkpF?=
 =?utf-8?B?RDJCM0E4UGNWZWZ3Tnk5a0xNRkJPeWQwRmJLTHZXdkhDaythUGFqNTVSUk1h?=
 =?utf-8?B?dlZSVzltNWs1MXBqcHRkdm56VUtkRWtSSUpWcU1FK05oQ05tVk4wRzJRbnpH?=
 =?utf-8?B?TENudlFlMEpXUzQ3bVp4Mm04OHltS0JqMm4xeVFzMWN0S1VEaGtaT2NUOG4x?=
 =?utf-8?B?bW1kUm1aMWw4MWxVTEt1QWU3SkFmOWxBYitmZE1DQ0h6N2QvYUJaOHc4dnY5?=
 =?utf-8?B?TWdGOTZkdU9WZE05NnYyV1pVN0ptd2lMd29hckMzZ3pqREdBMmhnNjZVbXBV?=
 =?utf-8?B?NE91VnlkL0d2Ukl3dXhQWGpSajNDcHg4dDJTU0Z2STRsRHlaaCtTeVlMK3FC?=
 =?utf-8?B?VllZZ0lYYTc3TlpnS2FTa0hicW1LK0JQNitmUDFYaUlXTktLRTlHbVovaU9E?=
 =?utf-8?B?U0E3VVJkclhyVktZR29WSk5NaVFrUVhXUHpWTmxvMitmeDRFL0RHMVhrRmhJ?=
 =?utf-8?B?ODUvVDJqbTk3M0Jqdmg5UnNSVWoydVA1ZVZFR0o3S2Fvd0duaStVYXpPTGM0?=
 =?utf-8?B?b3Bwdlo5Rk1ZZVhRYXl0K1REWkFRdUxFYUE0dEt1MFArQnV4bWwya1NFMzNY?=
 =?utf-8?B?NDROY25WL3JkcDVmdG5ZWnRQYmkxTDUvaVV6MW13UFlKdXBITHpwbXZDNTlW?=
 =?utf-8?B?UHk1amFxRURnWHhuWHdlSmR5SUtJTU95UWJHcjhVWUpBMW9LL3FxN29oc25Q?=
 =?utf-8?B?NVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b187458-725f-4cb6-5d6f-08dc6bb63416
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 May 2024 21:15:48.3055
 (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: tgIGrd35vITmkfLouzy0s3kG07y+7jiw26ddb43pPyjroX/Gew+qL1TUOTWMODNWAN8b84+arYNl4AJvkuktuXr8iA/koxTAUQ/+Uy0k4J8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6218
X-OriginatorOrg: intel.com

Hi Dave,

On 5/3/2024 7:53 AM, Dave Martin wrote:
> On Thu, May 02, 2024 at 10:52:15AM -0700, Reinette Chatre wrote:
>> 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().
> 
> Ah, right.  Found it.  I guess that works.
> 
> On a (sort of) related point, are there any concerns about namespace
> clashes in resctrlfs?  This looks like a potential issue for the resctrl
> top-level directory at least.
> 
> It's not clear to me how userspace can pick a name for a resctrl group
> that is guaranteed not to clash with the name of one of resctrl's own
> files in a future kernel.
> 
> (Note, this is nothing to do with series; I haven't been sure where to
> fit this into the dicsussion...)

It is not obvious to me what scenario you have in mind. Could you please
give an example?

> 
>>
>>>
>>>> +
>>>> +	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/
> 
> There, I was asking about "_" meaning "no flags" in "=_".

Apologies. I did not notice the difference. Yes, I agree, "_" is
not expected to be an operator.

Reinette