Re: [PATCH v12 05/31] x86,fs/resctrl: Refactor domain create/remove using struct rdt_domain_hdr

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

Hi Tony,

On 10/13/25 3:33 PM, Tony Luck wrote:
> Up until now, all monitoring events were associated with the L3 resource and it
> made sense to use the L3 specific "struct rdt_mon_domain *" argument to functions
> operating on domains.
> 
> Telemetry events will be tied to a new resource with its instances represented
> by a new domain structure that, just like struct rdt_mon_domain, starts with
> the generic struct rdt_domain_hdr.
> 
> Prepare to support domains belonging to different resources by changing the
> calling convention of functions operating on domains.  Pass the generic header
> and use that to find the domain specific structure where needed.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

...> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index f248eaf50d3c..8c07a9810706 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -547,11 +547,18 @@ struct rdt_domain_hdr *resctrl_find_domain(struct list_head *h, int id,
>  }
>  
>  void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> -		    struct rdt_mon_domain *d, struct rdtgroup *rdtgrp,
> +		    struct rdt_domain_hdr *hdr, struct rdtgroup *rdtgrp,
>  		    cpumask_t *cpumask, int evtid, int first)
>  {
> +	struct rdt_mon_domain *d = NULL;
>  	int cpu;
>  
> +	if (hdr) {
> +		if (!domain_header_is_valid(hdr, RESCTRL_MON_DOMAIN, RDT_RESOURCE_L3))
> +			return;
> +		d = container_of(hdr, struct rdt_mon_domain, hdr);
> +	}
> +
>  	/* When picking a CPU from cpu_mask, ensure it can't race with cpuhp */
>  	lockdep_assert_cpus_held();
>  

Please keep lockdep_assert_cpus_held() at beginning of the function.

> @@ -598,7 +605,6 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
>  	enum resctrl_event_id evtid;
>  	struct rdt_domain_hdr *hdr;
>  	struct rmid_read rr = {0};
> -	struct rdt_mon_domain *d;
>  	struct rdtgroup *rdtgrp;
>  	int domid, cpu, ret = 0;
>  	struct rdt_resource *r;
> @@ -623,6 +629,8 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
>  	r = resctrl_arch_get_resource(resid);
>  
>  	if (md->sum) {
> +		struct rdt_mon_domain *d;
> +
>  		/*
>  		 * This file requires summing across all domains that share
>  		 * the L3 cache id that was provided in the "domid" field of the

This hunk is unlike any other in this patch. To me the pattern of changes found in this
patch is that functions modified used to obtain struct rdt_mon_domain as parameter and
after this change these functions obtain struct rdt_domain_hdr as parameter and need to
use the container_of() to obtain the needed struct rdt_mon_domain. This hunk stands out
in that it just blindly assumes that the domain obtained by traversing rdt_resource::mon_domains
is an L3 domain.

How is this hunk relevant to this patch? 

From what I can tell the above two hunks need to be separated into a new patch and merged
with the unrelated changes added to patch #13 after it was reviewed.

Reinette