linux-kernel - Re: [PATCH v4 12/31] fs/resctrl: Improve handling for events that can be read from any CPU

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: <e818906f-b03a-474b-8a6b-d291cf1a74fe@intel.com>
Date: Tue, 13 May 2025 11:19:23 +0800
From: "Chen, Yu C" <yu.c.chen@...el.com>
To: Tony Luck <tony.luck@...el.com>, Reinette Chatre
	<reinette.chatre@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>, Fenghua Yu <fenghuay@...dia.com>, "Maciej
 Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
	<peternewman@...gle.com>, Babu Moger <babu.moger@....com>, Anil Keshavamurthy
	<anil.s.keshavamurthy@...el.com>, Dave Martin <Dave.Martin@....com>, "James
 Morse" <james.morse@....com>, Drew Fustini <dfustini@...libre.com>
Subject: Re: [PATCH v4 12/31] fs/resctrl: Improve handling for events that can
 be read from any CPU

Hi Tony,

On 4/29/2025 8:33 AM, Tony Luck wrote:
> Resctrl file system code was built with the assumption that monitor
> events can only be read from a CPU in the cpumast_t set for each
> domain. This was true for x86 events accessed with an MSR interface,
> but may not be true for other access methods such as MMIO.
> 
> Add a flag to each instance of struct mon_evt that can be set by
> architecture code to indicate there is no restriction on which
> CPU can read the event counter.
> 
> Change struct mon_data and struct rmid_read to have a pointer to
> the struct mon_evt instead of the event id.
> 
> Add an extra argument to resctrl_enable_mon_event() so architecture
> code can indicate which events can be read on any CPU when enabling
> the event.
> 
> Bypass all the smp_call*() code for events that can be read on any CPU
> and call mon_event_count() directly from mon_event_read().
> 
> Skip checks in __mon_event_count() that the read is being done from
> a CPU in the correct domain or cache scope.
> 

Since __mon_event_count() was supposed to run in atomic context, the
smp_processor_id() would not report any warning previously. After
this change, if the evt->any_cpu is true, we read the telemetry counter 
directly without IPI involved and in non-atomic context, we might
get warning like below:
BUG: using smp_processor_id() in preemptible [00000000] code: mount/1595
caller is __mon_event_count+0x2e/0x1e0
2483 [ 2095.332850] Call Trace:
2484 [ 2095.332861]  <TASK>
2485 [ 2095.332872]  dump_stack_lvl+0x55/0x70
2486 [ 2095.332887]  check_preemption_disabled+0xbf/0xe0
2487 [ 2095.332902]  __mon_event_count+0x2e/0x1e0
2488 [ 2095.332918]  mon_event_count+0x2a/0xa0
2489 [ 2095.332934]  mon_add_all_files+0x202/0x270
2490 [ 2095.332953]  mkdir_mondata_subdir+0x1bf/0x1e0
2491 [ 2095.332970]  ? kcore_update_ram.isra.0+0x270/0x270
2492 [ 2095.332985]  mkdir_mondata_all+0x9d/0x100
2493 [ 2095.333000]  rdt_get_tree+0x336/0x5d0
2494 [ 2095.333014]  vfs_get_tree+0x26/0xf0
2495 [ 2095.333028]  do_new_mount+0x186/0x350
2496 [ 2095.333044]  __x64_sys_mount+0x101/0x130
2497 [ 2095.333061]  do_syscall_64+0x54/0xd70
2498 [ 2095.333075]  entry_SYSCALL_64_after_hwframe+0x76/0x7e

Maybe avoid getting the CPU at all in __mon_event_count() if
evt->any_cpu is true?

thanks,
Chenyu
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index d9364bee486e..32385c811a92 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -358,12 +358,15 @@ static struct mbm_state *get_mbm_state(struct 
rdt_l3_mon_domain *d, u32 closid,

  static int __mon_event_count(u32 closid, u32 rmid, struct rmid_read *rr)
  {
-       int cpu = smp_processor_id();
         struct rdt_l3_mon_domain *d;
         struct mbm_state *m;
-       int err, ret;
+       int err, ret, cpu;
         u64 tval = 0;

+       /*only CPU sensitive event read cares about which CPU to read 
from */
+       if (!rr->evt->any_cpu)
+               cpu = smp_processor_id();

tele