| 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
| ||
|
Message-ID: <611ec112-02ee-492f-baef-270edbd08957@linux.alibaba.com>
Date: Fri, 12 Sep 2025 17:51:45 +0800
From: qinyuntan <qinyuntan@...ux.alibaba.com>
To: Reinette Chatre <reinette.chatre@...el.com>, tony.luck@...el.com,
james.morse@....com, Dave.Martin@....com, babu.moger@....com, bp@...en8.de,
tglx@...utronix.de, dave.hansen@...ux.intel.com
Cc: x86@...nel.org, hpa@...or.com, peternewman@...gle.com,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH] fs/resctrl: Eliminate false positive lockdep warning when
reading SNC counters
Hi Reinette,
Thank you for reporting this issue, and my apologies for the delayed
reply :).
I was able to reproduce the same lockdep warning on my local SNC-3
system, and the warning disappears after applying your patch.
The change looks good to me.
Reviewed-by: Qinyun Tan <qinyuntan@...ux.alibaba.com>
Thanks,
On 9/9/25 6:15 AM, Reinette Chatre wrote:
> Running resctrl_tests on an SNC-2 system with lockdep debugging enabled
> triggers several warnings with following trace:
> WARNING: CPU: 0 PID: 1914 at kernel/cpu.c:528 lockdep_assert_cpus_held+0x8a/0xc0
> ...
> Call Trace:
> __mon_event_count
> ? __lock_acquire
> ? __pfx___mon_event_count
> mon_event_count
> ? __pfx_smp_mon_event_count
> smp_mon_event_count
> smp_call_on_cpu_callback
> <snip>
>
> get_cpu_cacheinfo_level() called from __mon_event_count() requires CPU
> hotplug lock to be held. The hotplug lock is indeed held during this time,
> as confirmed by the lockdep_assert_cpus_held() within mon_event_read()
> that calls mon_event_count() via IPI, but the lockdep tracking is not able
> to follow the IPI.
>
> Fresh CPU cache information via get_cpu_cacheinfo_level() from
> __mon_event_count() was added to support the fix for the issue where
> resctrl inappropriately maintained links to L3 cache information that will
> be stale in the case when the associated CPU goes offline.
>
> Keep the cacheinfo ID in struct rdt_mon_domain to ensure that
> resctrl does not maintain stale cache information while CPUs can go
> offline. Return to using a pointer to the L3 cache information (struct
> cacheinfo) in struct rmid_read, rmid_read::ci. Initialize rmid_read::ci
> before the IPI where it is used. CPU hotplug lock is held across
> rmid_read::ci initialization and use to ensure that it points to accurate
> cache information.
>
> Fixes: 594902c986e2 ("x86,fs/resctrl: Remove inappropriate references to cacheinfo in the resctrl subsystem")
> Signed-off-by: Reinette Chatre <reinette.chatre@...el.com>
> ---
> fs/resctrl/ctrlmondata.c | 2 +-
> fs/resctrl/internal.h | 4 ++--
> fs/resctrl/monitor.c | 6 ++----
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index d98e0d2de09f..3c39cfacb251 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -625,11 +625,11 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
> */
> list_for_each_entry(d, &r->mon_domains, hdr.list) {
> if (d->ci_id == domid) {
> - rr.ci_id = d->ci_id;
> cpu = cpumask_any(&d->hdr.cpu_mask);
> ci = get_cpu_cacheinfo_level(cpu, RESCTRL_L3_CACHE);
> if (!ci)
> continue;
> + rr.ci = ci;
> mon_event_read(&rr, r, NULL, rdtgrp,
> &ci->shared_cpu_map, evtid, false);
> goto checkresult;
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index 0a1eedba2b03..9a8cf6f11151 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -98,7 +98,7 @@ struct mon_data {
> * domains in @r sharing L3 @ci.id
> * @evtid: Which monitor event to read.
> * @first: Initialize MBM counter when true.
> - * @ci_id: Cacheinfo id for L3. Only set when @d is NULL. Used when summing domains.
> + * @ci: Cacheinfo for L3. Only set when @d is NULL. Used when summing domains.
> * @err: Error encountered when reading counter.
> * @val: Returned value of event counter. If @rgrp is a parent resource group,
> * @val includes the sum of event counts from its child resource groups.
> @@ -112,7 +112,7 @@ struct rmid_read {
> struct rdt_mon_domain *d;
> enum resctrl_event_id evtid;
> bool first;
> - unsigned int ci_id;
> + struct cacheinfo *ci;
> int err;
> u64 val;
> void *arch_mon_ctx;
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index f5637855c3ac..7326c28a7908 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -361,7 +361,6 @@ static int __mon_event_count(u32 closid, u32 rmid, struct rmid_read *rr)
> {
> int cpu = smp_processor_id();
> struct rdt_mon_domain *d;
> - struct cacheinfo *ci;
> struct mbm_state *m;
> int err, ret;
> u64 tval = 0;
> @@ -389,8 +388,7 @@ static int __mon_event_count(u32 closid, u32 rmid, struct rmid_read *rr)
> }
>
> /* Summing domains that share a cache, must be on a CPU for that cache. */
> - ci = get_cpu_cacheinfo_level(cpu, RESCTRL_L3_CACHE);
> - if (!ci || ci->id != rr->ci_id)
> + if (!cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map))
> return -EINVAL;
>
> /*
> @@ -402,7 +400,7 @@ static int __mon_event_count(u32 closid, u32 rmid, struct rmid_read *rr)
> */
> ret = -EINVAL;
> list_for_each_entry(d, &rr->r->mon_domains, hdr.list) {
> - if (d->ci_id != rr->ci_id)
> + if (d->ci_id != rr->ci->id)
> continue;
> err = resctrl_arch_rmid_read(rr->r, d, closid, rmid,
> rr->evtid, &tval, rr->arch_mon_ctx);
Powered by blists - more mailing lists