lkml.org 
[lkml]   [2023]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH v4 12/24] x86/resctrl: Make resctrl_arch_rmid_read() retry when it is interrupted
From
Hi Peter,

On 06/06/2023 09:49, Peter Newman wrote:
> On Thu, May 25, 2023 at 8:03 PM James Morse <james.morse@arm.com> wrote:
>> +interrupted:
>> + am = get_arch_mbm_state(hw_dom, rmid, eventid);
>> + if (am)
>> + start_msr_val = atomic64_read(&am->prev_msr);
>> +
>> ret = __rmid_read(rmid, eventid, &msr_val);
>> if (ret)
>> return ret;
>>
>> am = get_arch_mbm_state(hw_dom, rmid, eventid);
>> if (am) {
>> - am->chunks += mbm_overflow_count(am->prev_msr, msr_val,
>> - hw_res->mbm_width);
>> - chunks = get_corrected_mbm_count(rmid, am->chunks);
>> - am->prev_msr = msr_val;
>> + old_msr_val = atomic64_cmpxchg(&am->prev_msr, start_msr_val,
>> + msr_val);
>> + if (old_msr_val != start_msr_val)
>> + goto interrupted;
>> +
>> + chunks = mbm_overflow_count(start_msr_val, msr_val,
>> + hw_res->mbm_width);
>> + atomic64_add(chunks, &am->chunks);
>> +
>> + chunks = get_corrected_mbm_count(rmid,
>> + atomic64_read(&am->chunks));
>> } else {
>> chunks = msr_val;
>> }
>
> It looks like if __rmid_read() is interrupted by an occupancy counter
> read between writing QM_EVTSEL and reading QM_CTR, it will not perform
> any update to am->prev_msr, and the interrupted read will return the
> same counter value as in the interrupting read.

Yup, that's a problem. I was only looking at the mbm state in memory, not the CPU register.
I think the fix is to read back QM_EVTSEL after reading QM_CTR. I'll do this in
__rmid_read() to avoid returning -EINTR. It creates two retry loops which is annoying, but
making the window larger means you're more likely to see false positives.

----------------------------%<----------------------------
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl
/monitor.c
index e24390d2e661..aeba035bb680 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -101,6 +101,7 @@ static inline u64 get_corrected_mbm_count(u32 rmid, unsigned
long val)

static int __rmid_read(u32 rmid, enum resctrl_event_id eventid, u64 *val)
{
+ u32 _rmid, _eventid;
u64 msr_val;

/*
@@ -110,9 +111,15 @@ static int __rmid_read(u32 rmid, enum resctrl_event_id eventid, u64 *val)
* IA32_QM_CTR.data (bits 61:0) reports the monitored data.
* IA32_QM_CTR.Error (bit 63) and IA32_QM_CTR.Unavailable (bit 62)
* are error bits.
+ * QM_EVTSEL is re-read to detect if this function was interrupted by
+ * another call, meaning the QM_CTR value may belong to a different
+ * event.
*/
- wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
- rdmsrl(MSR_IA32_QM_CTR, msr_val);
+ do {
+ wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
+ rdmsrl(MSR_IA32_QM_CTR, msr_val);
+ rdmsr(MSR_IA32_QM_EVTSEL, _eventid, _rmid);
+ } while (eventid != _eventid || rmid != _rmid);

if (msr_val & RMID_VAL_ERROR)
return -EIO;
----------------------------%<----------------------------

Thanks!

James

\
 
 \ /
  Last update: 2023-06-06 19:05    [W:0.386 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site