Received-SPF: pass (google.com: domain of linux-kernel+bounces-166826-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <30f2fb65-10ed-4d60-aadf-6727f7a6e404@amd.com>
Date: Thu, 2 May 2024 13:11:32 -0500
User-Agent: Mozilla Thunderbird
Reply-To: babu.moger@amd.com
Subject: Re: [RFC PATCH v3 17/17] x86/resctrl: Introduce interface to modify
 assignment states of the groups
To: Reinette Chatre <reinette.chatre@intel.com>,
 Dave Martin <Dave.Martin@arm.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>
 <4875492c-13cd-4a77-a42f-243fe0f868a2@intel.com>
Content-Language: en-US
From: "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <4875492c-13cd-4a77-a42f-243fe0f868a2@intel.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?LzZsR2IyK0ZROERnL2NNNzJMaTZjaUluVUNTem5qdzY3b294Y3lPMG1SRWFl?=
 =?utf-8?B?dkN1UE9CK21vQnpjVUlEQlpIeWlaT3IzaEk2czVndk5lQ0lJdW1DZWhhZ2ky?=
 =?utf-8?B?ZzhJNlM1STlkYkVkL0NvOFJnV2FxRlc5VHFnMHJlaTdUNm5IcUtoWWM0QXkx?=
 =?utf-8?B?elVoSkFxV3JEVXJyTnMxcUI5ZERweTE4QWJseVl0WXlwWUtGcldYK09EVUla?=
 =?utf-8?B?S2x4cEo3VnQzSWRST2NKQS9mUURPWk5mWS9lcXlsT1loeXVpTHF1S0JhL0Va?=
 =?utf-8?B?OERSaEppUXAzMXB4Qk9hamxqVmlRZnJuTUZtdHFVNzdNUUtPMzc0VXZuaUUv?=
 =?utf-8?B?M0tpeWNSM296NWNURzFLNlFUbzdwNlF2K205bkl4RzMweVNwMzNncEcwRTIy?=
 =?utf-8?B?RHZOMWRMNkFvQW5EZXZZRnM1dllKenFjZjNzUjZHaG95Ly9MaTBraVVSMThW?=
 =?utf-8?B?UVNQcWtIK05xRnpMT3NFYm5CcjlJUG5KQ3dxTmdLOVR2bE12OUVqcmg4NVdk?=
 =?utf-8?B?TGRkL3NTSkFSYXJUTERNcFR2UW5aNjloK1BGV0tvaVgxTVZFNHNWbFdPaGE1?=
 =?utf-8?B?ZE1FTXBQMGprUWJBMmFqanYwaDRuUEFldnlBTUwwenYrYWJLUUlpTVBPZVh3?=
 =?utf-8?B?UkZQOVhIMWNvQS84OEc1TnA0MjhaM3F6NkxkY25hclZZeEZIQUViYmF2WXg1?=
 =?utf-8?B?WE51MEVvaFdIQ1MwdWdheHBFWUlDV1Z2N1Q4UzVVa1ZoNXh3c3NjV2N2dzBM?=
 =?utf-8?B?QkltdlQ5SHFiZ0Z2ckVSSmZNaVpML1A4OGQ0NGFuRDl1RGlQWUE1RmhuL2xH?=
 =?utf-8?B?c25DbVlyb0x0TjVFZmVRbGxaaTdxU1ZtMXBTR2hqVjlXd1RZOENINFhRZSsy?=
 =?utf-8?B?UWpqRi91VTFYTWh3Tkw3c2xNeXp0SEZ2VlRkd0pyS0JVTXFmS0pnKzBMOTM5?=
 =?utf-8?B?NkxjemtLQ3BmR3R6d3MyWWZnTEw1aXJObnNaQ1N1M0l3RFBTdEQxdEN5REFr?=
 =?utf-8?B?L1BlV1A5ODFqVm5PMXRLa1N4TWhLakR5TFd6SjRtVm1wdWcxVWFJaUhyNVNk?=
 =?utf-8?B?eWpvcEdFMGZKZHFlTVNzaDI3Y2JWVmd4TzhKNnRNMFFTTGRjTDl6UVRVQXE1?=
 =?utf-8?B?VCtZRDBadGNWdXZsWmJ6dEFScTNyQWF2M2VpaGFrTERIdGlLdXVaSmVmQTBh?=
 =?utf-8?B?R3VITy9YazF2SUIzTm1LS0JISk8wMDh2U2JhWFRud0xHLzU5Q2haT1l4Rytn?=
 =?utf-8?B?cXhsNXJWdndUcTFwOUVVLzZWWnFGNUFKTC8wd2k0SVpnZ0NBWlJtN3BwMWl2?=
 =?utf-8?B?M0FndGM0b0s4RTN4U3VGT0tZbG1IV1c2VlFoNE1lRDc3NmZ5RENUSEl0aUZ4?=
 =?utf-8?B?VGVuamwvK3huMVExcTdnODdNd3M2U3c5Y1ZZd1hqbWRKWDlVa3NGbVJFS3k5?=
 =?utf-8?B?VUpOMXZxQk9TWE4xeUhXQ0N1MjE2bkRSZ3VQSXZxdkhFU2dmdHNhQ3c2UXhL?=
 =?utf-8?B?QnA4Sk9Jdkt4SzZOZnFLNld3eU1sdFhCUzQyRXdyZTJIWGcrWlZFbDNrZDhC?=
 =?utf-8?B?RFdiUTJtckZKaWs3UmZhNXVuTXVqTVdvL3hWMDE0bnR5aEtvU2p0NjlGRWVU?=
 =?utf-8?B?MWNaVU5IT3hHRGZSVmtESmJJYVJTRUplcEtFYytHZ3Z0dGxkdHJKYmNPL3FX?=
 =?utf-8?B?alhtZGZvT011ZFpNSWEvV0s3ZTRiaXl2MFdCQitUQUZ4TVExWXM0aGtSZ2VI?=
 =?utf-8?B?eW5zYmZvZmZ0M0VxNUlVZVd2WDFHYmRxNkRPSEE3RXJ2QnVyK2Z4bjBzQVY0?=
 =?utf-8?B?K2R4VTdYNkN1ZzkzOEppaWUrTGxhb1U1ZTFILzhVM2tEU1l3aGhRQUdSMnpH?=
 =?utf-8?B?UUF5WWdaTWJGVlJIQWtrNEZKZ05lNTR3UEE3NERheHJPejg4T3VDazltR1Fi?=
 =?utf-8?B?NGtKTXN1eU5xTEJ1UG1WRTdSUUJLUlJhZU1HbGk5bmRlQjUxUHdhZGZvem1W?=
 =?utf-8?B?MmtFQnpOVWMydi9nUGlZM29OSUVRQnZiUndkWi81MCtEdTVFTGppb0hHUmsx?=
 =?utf-8?B?Q1k4RWdRYVBwRWt3anlvN0dab21MWVpXMk1FT0VvNktXbU93MHo0V1JEVXRI?=
 =?utf-8?Q?0J7w=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0d08f2f8-3772-490c-f846-08dc6ad34eb8
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2024 18:11:37.2666
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 0Fg8lrPbbeS7hDT3qJNNQq8bXNck4Im6MHscfnT7l7DM8vgFq9O1bLYpjCromn2t
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR12MB8187

Hi Dave,


On 5/2/24 12:52, 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?

That is correct. I will add a generic format before this description and
then add these 4 cases. That way it will be more clear.


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

-- 
Thanks
Babu Moger