Messages in this thread | | | Date | Mon, 14 Aug 2023 16:58:21 -0700 | Subject | Re: [PATCH v5 06/24] x86/resctrl: Track the number of dirty RMID a CLOSID has | From | Fenghua Yu <> |
| |
Hi, James,
On 7/28/23 09:42, James Morse wrote: > MPAM's PMG bits extend its PARTID space, meaning the same PMG value can be > used for different control groups. > > This means once a CLOSID is allocated, all its monitoring ids may still be > dirty, and held in limbo. > > Keep track of the number of RMID held in limbo each CLOSID has. This will > allow a future helper to find the 'cleanest' CLOSID when allocating. > > The array is only needed when CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID is > defined. This will never be the case on x86. > > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v4: > * Moved closid_num_dirty_rmid[] update under entry->busy check > * Take the mutex in dom_data_init() as the caller doesn't. > --- > arch/x86/kernel/cpu/resctrl/monitor.c | 49 +++++++++++++++++++++++---- > 1 file changed, 42 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c > index de91ca781d9f..44addc0126fc 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -43,6 +43,13 @@ struct rmid_entry { > */ > static LIST_HEAD(rmid_free_lru); > > +/** > + * @closid_num_dirty_rmid The number of dirty RMID each CLOSID has. > + * Only allocated when CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID is defined. > + * Indexed by CLOSID. Protected by rdtgroup_mutex. > + */ > +static int *closid_num_dirty_rmid; > + > /** > * @rmid_limbo_count count of currently unused but (potentially) > * dirty RMIDs. > @@ -285,6 +292,17 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct rdt_domain *d, > return 0; > } > > +static void limbo_release_entry(struct rmid_entry *entry) > +{ > + lockdep_assert_held(&rdtgroup_mutex); > + > + rmid_limbo_count--; > + list_add_tail(&entry->list, &rmid_free_lru); > + > + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) > + closid_num_dirty_rmid[entry->closid]--; > +} > + > /* > * Check the RMIDs that are marked as busy for this domain. If the > * reported LLC occupancy is below the threshold clear the busy bit and > @@ -321,10 +339,8 @@ void __check_limbo(struct rdt_domain *d, bool force_free) > > if (force_free || !rmid_dirty) { > clear_bit(idx, d->rmid_busy_llc); > - if (!--entry->busy) { > - rmid_limbo_count--; > - list_add_tail(&entry->list, &rmid_free_lru); > - } > + if (!--entry->busy) > + limbo_release_entry(entry); > } > cur_idx = idx + 1; > } > @@ -391,6 +407,8 @@ static void add_rmid_to_limbo(struct rmid_entry *entry) > u64 val = 0; > u32 idx; > > + lockdep_assert_held(&rdtgroup_mutex); > + > idx = resctrl_arch_rmid_idx_encode(entry->closid, entry->rmid); > > entry->busy = 0; > @@ -416,9 +434,11 @@ static void add_rmid_to_limbo(struct rmid_entry *entry) > } > put_cpu(); > > - if (entry->busy) > + if (entry->busy) { > rmid_limbo_count++; > - else > + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) > + closid_num_dirty_rmid[entry->closid]++; > + } else > list_add_tail(&entry->list, &rmid_free_lru);
Unbalanced braces in if-else. Need to add braces in "else".
> } > > @@ -782,13 +802,28 @@ void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms) > static int dom_data_init(struct rdt_resource *r) > { > u32 idx_limit = resctrl_arch_system_num_rmid_idx(); > + u32 num_closid = resctrl_arch_get_num_closid(r); > struct rmid_entry *entry = NULL; > u32 idx; > int i; > > + if (IS_ENABLED(CONFIG_RESCTRL_RMID_DEPENDS_ON_CLOSID)) { > + int *tmp; > + > + tmp = kcalloc(num_closid, sizeof(int), GFP_KERNEL); > + if (!tmp) > + return -ENOMEM; > + > + mutex_lock(&rdtgroup_mutex); > + closid_num_dirty_rmid = tmp; > + mutex_unlock(&rdtgroup_mutex); > + } > + > rmid_ptrs = kcalloc(idx_limit, sizeof(struct rmid_entry), GFP_KERNEL); > - if (!rmid_ptrs) > + if (!rmid_ptrs) { > + kfree(closid_num_dirty_rmid); > return -ENOMEM; > + } > > for (i = 0; i < idx_limit; i++) { > entry = &rmid_ptrs[i];
Thanks.
-Fenghua
| |