Re: [PATCH v6 03/22] x86/resctrl: Consolidate monitoring related data from rdt_resource
From: Moger, Babu
Date: Mon Aug 19 2024 - 10:46:21 EST
Hi Reinette,
On 8/16/24 16:29, Reinette Chatre wrote:
> Hi Babu,
>
> On 8/6/24 3:00 PM, Babu Moger wrote:
>> The cache allocation and memory bandwidth allocation feature properties
>> are consolidated into cache and membw structures respectively.
>
> "are consolidated into cache and membw structures respectively" ->
> "are consolidated into struct resctrl_cache and struct resctrl_membw
> respectively"
Sure.
>
>>
>> In preparation for more monitoring properties that will clobber the
>> existing resource struct more, re-organize the monitoring specific
>> properties to also be in a separate structure.
>>
>> Suggested-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>
> ...
>
>> @@ -182,12 +182,21 @@ enum resctrl_scope {
>> RESCTRL_L3_NODE,
>> };
>> +/**
>> + * struct resctrl_mon - Monitoring related data
>
> To capture that this is not global monitoring data but instead
> resource specific:
> "Monitoring related data" -> "Monitoring related data of a resctrl resource"
Sure.
>
>> + * @num_rmid: Number of RMIDs available
>> + * @evt_list: List of monitoring events
>> + */
>> +struct resctrl_mon {
>> + int num_rmid;
>> + struct list_head evt_list;
>> +};
>> +
>> /**
>> * struct rdt_resource - attributes of a resctrl resource
>> * @rid: The index of the resource
>> * @alloc_capable: Is allocation available on this machine
>> * @mon_capable: Is monitor feature available on this machine
>> - * @num_rmid: Number of RMIDs available
>> * @ctrl_scope: Scope of this resource for control functions
>> * @mon_scope: Scope of this resource for monitor functions
>> * @cache: Cache allocation related data
>> @@ -199,7 +208,6 @@ enum resctrl_scope {
>> * @default_ctrl: Specifies default cache cbm or memory B/W percent.
>> * @format_str: Per resource format string to show domain value
>> * @parse_ctrlval: Per resource function pointer to parse control
>> values
>> - * @evt_list: List of monitoring events
>> * @fflags: flags to choose base and info files
>> * @cdp_capable: Is the CDP feature available on this resource
>> */
>
> Please add a kernel-doc entry for the new member.
Yes. Missed it.
>
>> @@ -207,11 +215,11 @@ struct rdt_resource {
>> int rid;
>> bool alloc_capable;
>> bool mon_capable;
>> - int num_rmid;
>> enum resctrl_scope ctrl_scope;
>> enum resctrl_scope mon_scope;
>> struct resctrl_cache cache;
>> struct resctrl_membw membw;
>> + struct resctrl_mon mon;
>> struct list_head ctrl_domains;
>> struct list_head mon_domains;
>> char *name;
>> @@ -221,7 +229,6 @@ struct rdt_resource {
>> int (*parse_ctrlval)(struct rdt_parse_data *data,
>> struct resctrl_schema *s,
>> struct rdt_ctrl_domain *d);
>> - struct list_head evt_list;
>> unsigned long fflags;
>> bool cdp_capable;
>> };
>
> Reinette
>
--
Thanks
Babu Moger