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