Re: [PATCH v7 08/14] fs,x86,mpam/resctrl: Handle change in number of RMIDs on each mount

[Luck, Tony]

On Tue, Jun 09, 2026 at 04:35:42PM -0700, Reinette Chatre wrote:
> Hi Tony,
> 
> On 6/9/26 2:58 PM, Luck, Tony wrote:
> > On Mon, Jun 08, 2026 at 04:21:10PM -0700, Reinette Chatre wrote:
> >> Hi Tony,
> >>
> > Changed the Subject: tag to "mpam,x86,fs/resctrl" to match what you used
> > for the RFC discussion. I think the implied rule is "architectures first
> > in alphabetical order, filesystem last".
> 
> After seeing the RFC discussions I do not think what I did in PoC is ideal.
> MPAM is the name of the Arm feature and soon there will also be RISC-V with its
> "Ssqosid" and "QBQRI" features. Since we currently have x86 as established
> prefix for the PQoS and RDT features it may be more appropriate and simpler
> to instead use something like "arm,riscv,x86,fs/resctrl" if such global
> change is ever needed? It still follows your implied rule of "architectures
> first in alphabetical order" ... but it instead actually uses the architecture
> names and not a mix of feature and architecture names. I am not dictating here
> and open to suggestions.

This sounds good. I'll switch to "arm,x86,fs/resctrl" for the patches in
this series that touch three places.
> 
> > 
> >> On 6/1/26 12:56 PM, Tony Luck wrote:
> ...
> >>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> >>> index 327e7a863614..b374e2f84a75 100644
> >>> --- a/fs/resctrl/monitor.c
> >>> +++ b/fs/resctrl/monitor.c
> >>> @@ -115,10 +115,17 @@ static inline struct rmid_entry *__rmid_entry(u32 idx)
> >>>  
> >>>  static void limbo_release_entry(struct rmid_entry *entry)
> >>>  {
> >>> +	u32 idx_limit = resctrl_arch_system_num_rmid_idx();
> >>
> >> Having the code switch idx_limit to sometimes be resctrl_arch_system_num_rmid_idx()
> >> and other times resctrl_arch_system_max_rmid_idx() makes this change difficult to
> >> follow. I think it will help to use different variable names to differentiate 
> >> the context in which it is being used and not leave reader trying to understand
> >> why there are *two* limits.
> > 
> > I'll change the local variable name to max_idx_limit when dealing with
> > the max value rather than the current. setup_rmid_lru_list() will have
> > both variables since max_idx_limit is used for allocation, and idx_limit
> > to add the right number to rmid_free_lru list. Added a comment to
> > __check_limbo() explaining why max RMID is used there. I think the other
> > spots are easy to see why.
> What do you think of "min_idx_limit" instead of "idx_limit" to complement
> the "max_idx_limit" while reflecting that it is the limit that is common 
> (hence "minimum") among all monitoring resources?

min_idx_limit nicely matches max_idx_limit. current_idx_limit could also
be a contender (though it is longer, and "current" might not obviously
refer to the mount/unmount cycle).

I think I'll switch to min_idx_limit.
> 
> Reinette