resctrl assumes that monitor events can only be read from a CPU in the
cpumask_t set of each domain.  This is true for x86 events accessed
with an MSR interface, but may not be true for other access methods such
as MMIO.

Introduce and use flag mon_evt::any_cpu, settable by architecture, that
indicates there are no restrictions on which CPU can read that event.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 include/linux/resctrl.h            | 2 +-
 fs/resctrl/internal.h              | 2 ++
 arch/x86/kernel/cpu/resctrl/core.c | 6 +++---
 fs/resctrl/ctrlmondata.c           | 6 ++++++
 fs/resctrl/monitor.c               | 3 ++-
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index a07542957e5a..702205505dc9 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -409,7 +409,7 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
 u32 resctrl_arch_system_num_rmid_idx(void);
 int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
 
-void resctrl_enable_mon_event(enum resctrl_event_id eventid);
+void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu);
 
 bool resctrl_is_mon_event_enabled(enum resctrl_event_id eventid);
 
diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
index 12a2ab7e3c9b..40b76eaa33d0 100644
--- a/fs/resctrl/internal.h
+++ b/fs/resctrl/internal.h
@@ -61,6 +61,7 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
  *			READS_TO_REMOTE_MEM) being tracked by @evtid.
  *			Only valid if @evtid is an MBM event.
  * @configurable:	true if the event is configurable
+ * @any_cpu:		true if the event can be read from any CPU
  * @enabled:		true if the event is enabled
  */
 struct mon_evt {
@@ -69,6 +70,7 @@ struct mon_evt {
 	char			*name;
 	u32			evt_cfg;
 	bool			configurable;
+	bool			any_cpu;
 	bool			enabled;
 };
 
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 99d1048d29d1..78ad493dcc01 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -893,15 +893,15 @@ static __init bool get_rdt_mon_resources(void)
 	bool ret = false;
 
 	if (rdt_cpu_has(X86_FEATURE_CQM_OCCUP_LLC)) {
-		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID);
+		resctrl_enable_mon_event(QOS_L3_OCCUP_EVENT_ID, false);
 		ret = true;
 	}
 	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_TOTAL)) {
-		resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID);
+		resctrl_enable_mon_event(QOS_L3_MBM_TOTAL_EVENT_ID, false);
 		ret = true;
 	}
 	if (rdt_cpu_has(X86_FEATURE_CQM_MBM_LOCAL)) {
-		resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID);
+		resctrl_enable_mon_event(QOS_L3_MBM_LOCAL_EVENT_ID, false);
 		ret = true;
 	}
 	if (rdt_cpu_has(X86_FEATURE_ABMC))
diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index c3656812848b..883be6f0810f 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -574,6 +574,11 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
 		}
 	}
 
+	if (evt->any_cpu) {
+		mon_event_count(rr);
+		goto out_ctx_free;
+	}
+
 	cpu = cpumask_any_housekeeping(cpumask, RESCTRL_PICK_ANY_CPU);
 
 	/*
@@ -587,6 +592,7 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
 	else
 		smp_call_on_cpu(cpu, smp_mon_event_count, rr, false);
 
+out_ctx_free:
 	if (rr->arch_mon_ctx)
 		resctrl_arch_mon_ctx_free(r, evt->evtid, rr->arch_mon_ctx);
 }
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index 5cf928e10eaf..6eab98b47816 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -975,7 +975,7 @@ struct mon_evt mon_event_all[QOS_NUM_EVENTS] = {
 	},
 };
 
-void resctrl_enable_mon_event(enum resctrl_event_id eventid)
+void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu)
 {
 	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
 		return;
@@ -984,6 +984,7 @@ void resctrl_enable_mon_event(enum resctrl_event_id eventid)
 		return;
 	}
 
+	mon_event_all[eventid].any_cpu = any_cpu;
 	mon_event_all[eventid].enabled = true;
 }
 
-- 
2.51.0

Hi Tony,

On 10/29/25 9:20 AM, Tony Luck wrote:
> resctrl assumes that monitor events can only be read from a CPU in the
> cpumask_t set of each domain.  This is true for x86 events accessed
> with an MSR interface, but may not be true for other access methods such
> as MMIO.
> 
> Introduce and use flag mon_evt::any_cpu, settable by architecture, that
> indicates there are no restrictions on which CPU can read that event.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---

This implies a generic change while __l3_mon_event_count() cannot support it.
While I understand why cpu_on_correct_domain() was removed, the existing checking
requires DEBUG_PREEMPT to be set so I think it would be helpful if there is something
like a WARN_ON_ONCE(rr->evt->any_cpu) before calling __l3_mon_event_count() to help
catch issues.

Reinette

Hi Tony,

On 10/30/2025 12:20 AM, Tony Luck wrote:
> resctrl assumes that monitor events can only be read from a CPU in the
> cpumask_t set of each domain.  This is true for x86 events accessed
> with an MSR interface, but may not be true for other access methods such
> as MMIO.
> 
> Introduce and use flag mon_evt::any_cpu, settable by architecture, that
> indicates there are no restrictions on which CPU can read that event.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>

[snip]

> -void resctrl_enable_mon_event(enum resctrl_event_id eventid)
> +void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu)
>   {
>   	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
>   		return;
> @@ -984,6 +984,7 @@ void resctrl_enable_mon_event(enum resctrl_event_id eventid)
>   		return;
>   	}
>   
> +	mon_event_all[eventid].any_cpu = any_cpu;
>   	mon_event_all[eventid].enabled = true;
>   }
>   

It seems that cpu_on_correct_domain() was dropped, due to
the refactor of __mon_event_count() in patch 0006 means it is no
longer needed.  But we still invoke smp_processor_id() in preemptible
context in __l3_mon_event_count() before further checkings, which would
cause a warning.
[ 4266.361951] BUG: using smp_processor_id() in preemptible [00000000] 
code: grep/1603
[ 4266.363231] caller is __l3_mon_event_count+0x30/0x2a0
[ 4266.364250] Call Trace:
[ 4266.364262]  <TASK>
[ 4266.364273]  dump_stack_lvl+0x53/0x70
[ 4266.364289]  check_preemption_disabled+0xca/0xe0
[ 4266.364303]  __l3_mon_event_count+0x30/0x2a0
[ 4266.364320]  mon_event_count+0x22/0x90
[ 4266.364334]  rdtgroup_mondata_show+0x108/0x390
[ 4266.364353]  seq_read_iter+0x10d/0x450
[ 4266.364368]  vfs_read+0x215/0x330
[ 4266.364386]  ksys_read+0x6b/0xe0
[ 4266.364401]  do_syscall_64+0x57/0xd70

thanks,
Chenyu

On Thu, Oct 30, 2025 at 02:14:27PM +0800, Chen, Yu C wrote:
> Hi Tony,
> 
> On 10/30/2025 12:20 AM, Tony Luck wrote:
> > resctrl assumes that monitor events can only be read from a CPU in the
> > cpumask_t set of each domain.  This is true for x86 events accessed
> > with an MSR interface, but may not be true for other access methods such
> > as MMIO.
> > 
> > Introduce and use flag mon_evt::any_cpu, settable by architecture, that
> > indicates there are no restrictions on which CPU can read that event.
> > 
> > Signed-off-by: Tony Luck <tony.luck@intel.com>
> 
> [snip]
> 
> > -void resctrl_enable_mon_event(enum resctrl_event_id eventid)
> > +void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu)
> >   {
> >   	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
> >   		return;
> > @@ -984,6 +984,7 @@ void resctrl_enable_mon_event(enum resctrl_event_id eventid)
> >   		return;
> >   	}
> > +	mon_event_all[eventid].any_cpu = any_cpu;
> >   	mon_event_all[eventid].enabled = true;
> >   }
> 
> It seems that cpu_on_correct_domain() was dropped, due to
> the refactor of __mon_event_count() in patch 0006 means it is no
> longer needed.  But we still invoke smp_processor_id() in preemptible
> context in __l3_mon_event_count() before further checkings, which would
> cause a warning.
> [ 4266.361951] BUG: using smp_processor_id() in preemptible [00000000] code:
> grep/1603
> [ 4266.363231] caller is __l3_mon_event_count+0x30/0x2a0
> [ 4266.364250] Call Trace:
> [ 4266.364262]  <TASK>
> [ 4266.364273]  dump_stack_lvl+0x53/0x70
> [ 4266.364289]  check_preemption_disabled+0xca/0xe0
> [ 4266.364303]  __l3_mon_event_count+0x30/0x2a0
> [ 4266.364320]  mon_event_count+0x22/0x90
> [ 4266.364334]  rdtgroup_mondata_show+0x108/0x390
> [ 4266.364353]  seq_read_iter+0x10d/0x450
> [ 4266.364368]  vfs_read+0x215/0x330
> [ 4266.364386]  ksys_read+0x6b/0xe0
> [ 4266.364401]  do_syscall_64+0x57/0xd70

I didn't notice this in my testing. Is this in your region aware
tree? If you are still using RDT_RESOURCE_L3 then I can see how
you got this call trace.

Maybe you need to dig cpu_on_correct_domain() back up and apply
it to __l3_mon_event_count()?

-Tony

On 10/30/2025 11:54 PM, Luck, Tony wrote:
> On Thu, Oct 30, 2025 at 02:14:27PM +0800, Chen, Yu C wrote:
>> Hi Tony,
>>
>> On 10/30/2025 12:20 AM, Tony Luck wrote:
>>> resctrl assumes that monitor events can only be read from a CPU in the
>>> cpumask_t set of each domain.  This is true for x86 events accessed
>>> with an MSR interface, but may not be true for other access methods such
>>> as MMIO.
>>>
>>> Introduce and use flag mon_evt::any_cpu, settable by architecture, that
>>> indicates there are no restrictions on which CPU can read that event.
>>>
>>> Signed-off-by: Tony Luck <tony.luck@intel.com>
>>
>> [snip]
>>
>>> -void resctrl_enable_mon_event(enum resctrl_event_id eventid)
>>> +void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu)
>>>    {
>>>    	if (WARN_ON_ONCE(eventid < QOS_FIRST_EVENT || eventid >= QOS_NUM_EVENTS))
>>>    		return;
>>> @@ -984,6 +984,7 @@ void resctrl_enable_mon_event(enum resctrl_event_id eventid)
>>>    		return;
>>>    	}
>>> +	mon_event_all[eventid].any_cpu = any_cpu;
>>>    	mon_event_all[eventid].enabled = true;
>>>    }
>>
>> It seems that cpu_on_correct_domain() was dropped, due to
>> the refactor of __mon_event_count() in patch 0006 means it is no
>> longer needed.  But we still invoke smp_processor_id() in preemptible
>> context in __l3_mon_event_count() before further checkings, which would
>> cause a warning.
>> [ 4266.361951] BUG: using smp_processor_id() in preemptible [00000000] code:
>> grep/1603
>> [ 4266.363231] caller is __l3_mon_event_count+0x30/0x2a0
>> [ 4266.364250] Call Trace:
>> [ 4266.364262]  <TASK>
>> [ 4266.364273]  dump_stack_lvl+0x53/0x70
>> [ 4266.364289]  check_preemption_disabled+0xca/0xe0
>> [ 4266.364303]  __l3_mon_event_count+0x30/0x2a0
>> [ 4266.364320]  mon_event_count+0x22/0x90
>> [ 4266.364334]  rdtgroup_mondata_show+0x108/0x390
>> [ 4266.364353]  seq_read_iter+0x10d/0x450
>> [ 4266.364368]  vfs_read+0x215/0x330
>> [ 4266.364386]  ksys_read+0x6b/0xe0
>> [ 4266.364401]  do_syscall_64+0x57/0xd70
> 
> I didn't notice this in my testing. Is this in your region aware
> tree? If you are still using RDT_RESOURCE_L3 then I can see how
> you got this call trace.
> 

Yes, it was tested on the region aware tree.

> Maybe you need to dig cpu_on_correct_domain() back up and apply
> it to __l3_mon_event_count()?
> 

Got it, will do.

Thanks,
Chenyu