[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f5a80f3a-1aaa-4831-a9f1-7f3eca748dfe@intel.com>
Date: Fri, 3 Oct 2025 08:34:01 -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 v11 06/31] x86,fs/resctrl: Use struct rdt_domain_hdr when
reading counters
Hi Tony,
On 9/25/25 1:03 PM, Tony Luck wrote:
> Use a generic struct rdt_domain_hdr representing a generic domain
> header in struct rmid_read in order to support other telemetry events'
> domains besides an L3 one. Adjust the code interacting with it to the
> new struct layout.
I'd propose a small amend to be more specific and not assume reader knows
what rmid_read is used for:
struct rmid_read contains data passed around to read event counts. Use the
generic domain header struct rdt_domain_hdr in struct rmid_read in order to
support other telemetry events' domains besides an L3 one. Adjust the code
interacting with it to the new struct layout.
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index 3ceef35208be..7b9fc5d3bdc8 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -550,13 +550,8 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> struct rdt_domain_hdr *hdr, struct rdtgroup *rdtgrp,
> cpumask_t *cpumask, int evtid, int first)
> {
> - struct rdt_mon_domain *d;
> int cpu;
>
> - if (!domain_header_is_valid(hdr, RESCTRL_MON_DOMAIN, RDT_RESOURCE_L3))
> - return;
> - d = container_of(hdr, struct rdt_mon_domain, hdr);
> -
Problematic snippet removed here ...
> /* When picking a CPU from cpu_mask, ensure it can't race with cpuhp */
> lockdep_assert_cpus_held();
>
> @@ -566,7 +561,7 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> rr->rgrp = rdtgrp;
> rr->evtid = evtid;
> rr->r = r;
> - rr->d = d;
> + rr->hdr = hdr;
> rr->first = first;
> if (resctrl_arch_mbm_cntr_assign_enabled(r) &&
> resctrl_is_mbm_event(evtid)) {
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 4076336fbba6..32116361a5f6 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -159,7 +159,7 @@ void __check_limbo(struct rdt_mon_domain *d, bool force_free)
> break;
>
> entry = __rmid_entry(idx);
> - if (resctrl_arch_rmid_read(r, d, entry->closid, entry->rmid,
> + if (resctrl_arch_rmid_read(r, &d->hdr, entry->closid, entry->rmid,
> QOS_L3_OCCUP_EVENT_ID, &val,
> arch_mon_ctx)) {
> rmid_dirty = true;
> @@ -424,8 +424,12 @@ static int __mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
> int err, ret;
> u64 tval = 0;
>
> + if (!domain_header_is_valid(rr->hdr, RESCTRL_MON_DOMAIN, RDT_RESOURCE_L3))
> + return -EINVAL;
> + d = container_of(rr->hdr, struct rdt_mon_domain, hdr);
> +
... but now the problem is moved to __mon_event_count() where rr->hdr can be NULL and the
domain_header_is_valid() check is referencing NULL pointer when SNC is enabled?
Am I missing something here? Does this work on SNC?
Reinette
Powered by blists - more mailing lists