Messages in this thread | | | Date | Fri, 3 Mar 2023 18:33:36 +0000 | Subject | Re: [PATCH v2 02/18] x86/resctrl: Access per-rmid structures by index | From | James Morse <> |
| |
Hi Reinette,
On 02/02/2023 23:44, Reinette Chatre wrote: > On 1/13/2023 9:54 AM, James Morse wrote: >> Because of the differences between Intel RDT/AMD QoS and Arm's MPAM >> monitors, RMID values on arm64 are not unique unless the CLOSID is >> also included. Bitmaps like rmid_busy_llc need to be sized by the >> number of unique entries for this resource. >> >> Add helpers to encode/decode the CLOSID and RMID to an index. The >> domain's busy_rmid_llc and the rmid_ptrs[] array are then sized by > > busy_rmid_llc -> rmid_busy_llc ? > > Could you please also mention the MBM state impacted?
Yup, this paragraph reworded as: | Add helpers to encode/decode the CLOSID and RMID to an index. The | domain's rmid_busy__llc and the rmid_ptrs[] array are then sized by | index, as are the domain mbm_local and mbm_total arrays. | On x86, the index is always just the RMID, so all these structures | remain the same size. | | The index gives resctrl a unique value it can use to store monitor | values, and allows MPAM to decode the closid when reading the hardware | counters.
>> diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h >> index 52788f79786f..44d568f3577c 100644 >> --- a/arch/x86/include/asm/resctrl.h >> +++ b/arch/x86/include/asm/resctrl.h >> @@ -7,6 +7,13 @@ >> #include <linux/sched.h> >> #include <linux/jump_label.h> >> >> +/* >> + * This value can never be a valid CLOSID, and is used when mapping a >> + * (closid, rmid) pair to an index and back. On x86 only the RMID is >> + * needed. >> + */ >> +#define X86_RESCTRL_BAD_CLOSID ~0
> Should this be moved to previous patch where first usage of ~0 appears?
Makes sense,
> Also, not having a size creates opportunity for inconsistencies. How > about ((u32)~0) ?
Yes, the compilers secret expectations on sizes always catch me out!
>> + >> /** >> * struct resctrl_pqr_state - State cache for the PQR MSR >> * @cur_rmid: The cached Resource Monitoring ID >> @@ -94,6 +101,23 @@ static inline void resctrl_sched_in(void) >> __resctrl_sched_in(); >> } >> >> +static inline u32 resctrl_arch_system_num_rmid_idx(void) >> +{ >> + /* RMID are independent numbers for x86. num_rmid_idx==num_rmid */ >> + return boot_cpu_data.x86_cache_max_rmid + 1; >> +}
> It seems that this helper and its subsequent usage eliminates the > need for struct rdt_resource::num_rmid? Are any users left?
The only user in the filesystem parts of resctrl is rdt_num_rmids_show(), which exposes the value to user-space. The value is unfortunately meaningless on MPAM systems, but as its user-space ABI, it has to stay.
The remaining users in the x86 specific code: domain_add_cpu() continues to use r->num_rmid as the arch code can know its the same number as num_idx, and rdt_get_mon_l3_config() calculates the value.
Thanks,
James
| |