Re: [PATCH v19 09/20] x86/resctrl: Add new fields to struct rmid_read for summation of domains

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 5/28/24 3:19 PM, Tony Luck wrote:
> rdtgroup_mondata_show() calls mon_event_read() which calls
> mon_event_count() which packages up all the required details into an

No, mon_event_read() does the "packaging".

> rmid_read structure passed across the smp_call*() infrastructure.
> 
> Legacy files reporting for a single domain pass that domain in the
> rmid_read structure. Files that need to sum multiple domains have
> meta data that provides the L3 cache ID for domains that must be
> summed.
> 
> Add the sumdomains and cacheinfo fields to the rmid_read structure.

This just describes the code and that can be seen from patch. Please
check all changelogs in series for this.

> 
> Add kerneldoc comments for the rmid_read structure.

Same.

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>   arch/x86/kernel/cpu/resctrl/internal.h | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index eb70d3136ced..d8156d22cbdc 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -145,12 +145,28 @@ union mon_data_bits {
>   	} u;
>   };
>   
> +/**
> + * struct rmid_read - Data passed across smp_call*() to read event count
> + * @rgrp:	Resctrl group (provides RMID value)

Provides much more than RMID so either make that accurate or drop the annotation.

> + * @r:		Resource
> + * @d:		Domain
> + * @evtid:	Which monitor event to read
> + * @first:	When true this just requests initialization of an MBM counter

Seems strange. Perhaps just "Initializes MBM counter when true."

> + * @sumdomains:	When false just return monitor count from domain @d. When true,
> + *		sum all domains in @r sharing L3 @ci.id
> + * @ci:		See @sumdomains
> + * @err:	Used to return error indication
> + * @val:	Used to return value of event counter
> + * @arch_mon_ctx: hardware monitor allocated for this read request (MPAM only)

Stay consistent with descriptions starting with upper case.

> + */
>   struct rmid_read {
>   	struct rdtgroup		*rgrp;
>   	struct rdt_resource	*r;
>   	struct rdt_mon_domain	*d;
>   	enum resctrl_event_id	evtid;
>   	bool			first;
> +	bool			sumdomains;
> +	struct cacheinfo	*ci;
>   	int			err;
>   	u64			val;
>   	void			*arch_mon_ctx;

Reinette