Received-SPF: pass (google.com: domain of linux-kernel+bounces-175037-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <be20ffe1-ef8c-41fa-b359-9ebfaa326ebc@amd.com>
Date: Thu, 9 May 2024 17:34:02 -0500
User-Agent: Mozilla Thunderbird
Reply-To: babu.moger@amd.com
Subject: Re: [RFC PATCH v3 03/17] x86/resctrl: Detect Assignable Bandwidth
 Monitoring feature details
To: Reinette Chatre <reinette.chatre@intel.com>, corbet@lwn.net,
 fenghua.yu@intel.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
 dave.hansen@linux.intel.com
Cc: 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>
 <63134ddf18208bbd11bcec0ec75c33ef6e5635b2.1711674410.git.babu.moger@amd.com>
 <ac22b96d-c4bb-42d9-bac8-22b9e032b59e@intel.com>
 <80b33bc6-7b70-4853-92a0-718c2d8728d3@amd.com>
 <753844b6-b68d-4b05-89b0-3ac36e1c8564@intel.com>
Content-Language: en-US
From: "Moger, Babu" <babu.moger@amd.com>
In-Reply-To: <753844b6-b68d-4b05-89b0-3ac36e1c8564@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?M2Z0V3NFKzZNSUVWREU0bEpBYXJhMVdPeTd0ejNEZTdGRThrTjMweDVmekxI?=
 =?utf-8?B?VUQ1bkZuTTZxM0NZVXlmNmNCV1pvdjVTaGZkWnNDREJ1Y3ZVQU5Ca3JCcGZy?=
 =?utf-8?B?aFBJSUtrNGtjZlRIQ1lBV1V0OHRVU2RYZ1VmOE1MaU5Rbkc4czU3QkhJWWht?=
 =?utf-8?B?NTM4QTBZbWVEVFh4cE1KdUJRS1BJRHY5eCtvNDRMbmg1eEJ0WmZhS2xyYUh5?=
 =?utf-8?B?Zm5sbmhkYjVDYXFLV3ArUDhLYnpZeVdTY3hMQ1VlRzJIYmJraFFmSXVLRXNw?=
 =?utf-8?B?U3c0dGJ4MVYyZzZ0U28vNGlWV2NPYmNUd2tuUHlGZjY3ajNzSy9mVVZWOVBC?=
 =?utf-8?B?QWpYbThLMUlpTXBva3VCVVBVWWZOeXFYSGYyWE1DYXdaZStKSWk4Wm1iZG9V?=
 =?utf-8?B?WlBsNUtTOVNQc0s5dE9WQ2kwdWFaVUI4WEZJaGN1MTRhNWNNNmc3aGR6ZnJ0?=
 =?utf-8?B?YTVZUDdUSFp4cUorZkpjRmEvWDUvQi9JWnJxMjZDTUhBSnJsZHFoTXNjV1U4?=
 =?utf-8?B?dFZWSGtlaHNjTFRlY0g1Y0hKVnB4eFAwVmJ0ZXZKV3h1TFJvNlErZFlMR2hx?=
 =?utf-8?B?MFdHRjhqOWt3SWZvdFVmaWZXOHpiS2d0VTlSZ05BeE1lb1J1V2pzdngyRFZ4?=
 =?utf-8?B?OW9ZYSt4bGFvVFpML0N5SUFXbFdLR3luRGpsRnJPZmI3M3RoMFd1cTdUL3VZ?=
 =?utf-8?B?aTdVVnY3a3ZST000SEtEN3doTUhUSXBWTC85TjZNdEVJblRXcjBXVHQ3NGVl?=
 =?utf-8?B?SmVhZ1JrWEhiUnNlbDhNWkZDVU1RT0pWM0M2MG5GRDhCREhrUUFkcWNaVTZt?=
 =?utf-8?B?MXpDd2VDUnBkb3VZeEJRL1ZmTFZUVERjbnhqODdHbmhWNUpIbzlvUGhBM0ph?=
 =?utf-8?B?UjQrWnF6T1E5ekkwVGt6YmFKMitkQUFkUTNrK2U0KzZUK3kwQStBSXBNOXNo?=
 =?utf-8?B?ZnFaa29VT1lkRXQyR0xpdlVzUGJkblJ1Z1IySTU4VlBuSTFta2F3M3RZNCsw?=
 =?utf-8?B?b1l1TUZQMjFwTkoxOHlsajB4NmE2TGRQT25OYTZKSUN3Q2ozMXZHekM5bFRF?=
 =?utf-8?B?MXZGdDk1Y001elNwWUZFTkpPWXpnQi9wRzY3UHNIbUJabXk5Z1JxNFZacmtv?=
 =?utf-8?B?Z2VQT0dpc2krUXpLc0FZY011bmo3NFRtdHlHT2V0STVacXNEMHdmK1BVM3FD?=
 =?utf-8?B?cXlTVTNKNFRwR1VaQkxSRWNORmpoazY0a1lzQzI3V25SU0UrTU9jd2lxbksx?=
 =?utf-8?B?Z2dqdUl3TEFWNzAzcys1OVgyRmxQd2x6VjUxYVcwZXJmWm9VaVpReW9jS1Fh?=
 =?utf-8?B?dmdSU3IrTTBINmJSczcyK09iS1d3SytoRHBraTE4UktIZXZ0c3VuNE11WkRC?=
 =?utf-8?B?bzB6ckMzd3RtVXJybXY5KzZTMzZvcFlIYUVsODg3OGZIU0lPREFtd3pLQk5n?=
 =?utf-8?B?WWlQNzZvUjlCdEFFRDFRMFoyV1BTQVljY29SUmNqbENRNHAvQ1pqckpTQzFw?=
 =?utf-8?B?VjhzMFM5VWJ2clA0NnNuYlIrSENMN1k0RmIxa2pvT0J2WFVFZFhOaGxMdjNF?=
 =?utf-8?B?R0lTRlMwQnMvckNpTnRIbUh3YUlnYmdHdmpuTjZXQ0FrVzFZL2NSNmMxa3lC?=
 =?utf-8?B?dWtPb0tCOUJFbFlyUEZrUWdHMDd1YmQwNEI1YlJhWnR1U0xDd2pGb2l6NXRH?=
 =?utf-8?B?RzNYWjJCQmJWREJsNit6TUJMWXp6czI0cHBZZmJlT0lkN1NjTzlOVENxSHhl?=
 =?utf-8?B?T1JlUWJ3aHNjYis5RjB3UmZ0S21yWVcrUXhqbmFnQmd3VzlRSWNzK3gxYXFh?=
 =?utf-8?B?QnF0YlhvNURaczdIQnZtblZmekNsNE9ybm96UXlKVFA4RGlGa1N4TjRTTmRk?=
 =?utf-8?B?YnlRVkViQ1hyQm80NXNCUUtYZzQxRTd2ZWtGNU4rdU1aR0dwY0ZjMDJuZ1A1?=
 =?utf-8?B?RVQwYlRhS3JNeTNiY1ZlWmtGMERTbCthOFNRcVZuVWtRbStaU1ZNd2lscUZy?=
 =?utf-8?B?TFJZNWhlOEJ1akM5Z0NSMG85RjlNWXlGOVZZYUdCWmcxeFRFY211TGdtWUEw?=
 =?utf-8?B?Z2xWRXcyekxXRTA4bVdwUHUvd25xK3RacENTYjdsQjhvUnFxNmhJdmR2VjB5?=
 =?utf-8?Q?YBJc=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 046b6866-b9ed-4eb1-11d6-08dc707823a6
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2024 22:34:07.7771
 (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: ugZh7yhlfB3J64JxQJ0ahxRy/dgQPtWkAIxXVy4S0nF/VO+qzOPxvbNaAt1OKiO4
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB8161

Hi Reinette,

On 5/7/24 15:27, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/6/2024 12:09 PM, Moger, Babu wrote:
>> On 5/3/24 18:26, Reinette Chatre wrote:
>>> On 3/28/2024 6:06 PM, Babu Moger wrote:
> 
> ...
> 
>>>> + * @mbm_assign_cntrs:	Maximum number of assignable counters
>>>>   */
>>>>  struct rdt_resource {
>>>>  	int			rid;
>>>> @@ -188,6 +198,8 @@ struct rdt_resource {
>>>>  	struct list_head	evt_list;
>>>>  	unsigned long		fflags;
>>>>  	bool			cdp_capable;
>>>> +	bool			mbm_assign_capable;
>>>> +	u32                     mbm_assign_cntrs;
>>>>  };
>>>
>>> Please check tabs vs spaces (in this whole series please).
>>
>> Sure. Will do.
>>
>>>
>>> I'm thinking that a new "MBM specific" struct within
>>> struct rdt_resource will be helpful to clearly separate the MBM related
>>> data. This will be similar to struct resctrl_cache for 
>>> cache allocation and struct resctrl_membw for memory bandwidth
>>> allocation.
>>
>> Did you mean to move all the fields for monitoring to move to new struct?
>>
>> Struct resctrl_mbm {
>>           int                     num_rmid;
>>           bool                    mbm_assign_capable;
>>           u32                     mbm_assign_cntrs;
>> }:
>>
> 
> Indeed, so not actually MBM specific but monitoring specific as you state (with
> appropriate naming?). This is purely to help organize data within struct rdt_resource
> and (similar to struct resctrl_cache and struct resctrl_membw) not a new
> structure expected to be passed around. I think evt_list can also be a member.

How about this?

Lets keep assign_capable in main structure(like we have mon_capable) and
move rest of them into new structure.

Struct resctrl_mon {
           int                     num_rmid;
	   struct list_head        evt_list;
           u32                     num_assign_cntrs;
}:
-- 
Thanks
Babu Moger