linux-kernel - Re: [PATCH v12 05/31] x86,fs/resctrl: Refactor domain create/remove using struct rdt_domain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <224b358c-17e6-4e5d-935c-7559663ddf7d@intel.com>
Date: Wed, 22 Oct 2025 21:15:17 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghuay@...dia.com>, "Maciej
 Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
	<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
	<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
	<Dave.Martin@....com>, Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v12 05/31] x86,fs/resctrl: Refactor domain create/remove
 using struct rdt_domain_hdr

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@...el.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