Hi Reinette,
On 10/15/2024 10:29 PM, Reinette Chatre wrote:
Hi Babu,
On 10/9/24 10:39 AM, Babu Moger wrote:
The mbm_cntr_assign mode provides a limited number of hardware counters
that can be assigned to an RMID-event pair to monitor bandwidth while
assigned. If all counters are in use, the kernel will show an error
message: "Out of MBM assignable counters" when a new assignment is
requested. To make space for a new assignment, users must unassign an
already assigned counter.
Introduce an interface that allows for the unassignment of counter IDs
from both the group and the domain. Additionally, ensure that the global
counter is released if it is no longer assigned to any domains.
Needs imperative tone ... "Release the global counter ..."
sure.
Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
...
---
arch/x86/kernel/cpu/resctrl/internal.h | 2 +
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 56 ++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 900e18aea2c4..6f388d20fb22 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -717,6 +717,8 @@ int resctrl_arch_config_cntr(struct rdt_resource *r, struct rdt_mon_domain *d,
u32 cntr_id, bool assign);
int rdtgroup_assign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
struct rdt_mon_domain *d, enum resctrl_event_id evtid);
+int rdtgroup_unassign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
+ struct rdt_mon_domain *d, enum resctrl_event_id evtid);
void rdt_staged_configs_clear(void);
bool closid_allocated(unsigned int closid);
int resctrl_find_cleanest_closid(void);
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e4f628e6fe65..791258adcbda 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1945,6 +1945,62 @@ int rdtgroup_assign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
return ret;
}
+static bool mbm_cntr_assigned_to_domain(struct rdt_resource *r, u32 cntr_id)
+{
+ struct rdt_mon_domain *d;
+
+ list_for_each_entry(d, &r->mon_domains, hdr.list)
+ if (test_bit(cntr_id, d->mbm_cntr_map))
+ return 1;
+
+ return 0;
+}
+
+/*
+ * Unassign a hardware counter from the domain and the group.
Not sure ... maybe "Unassign a hardware counter associated with @evtid from
the domain and the group."?
ok.
+ * Counter will be unassigned in all the domains if rdt_mon_domain is NULL
Please use imperative tone: "Unassign the counter from all the domains ...."
ok.
+ * else the counter will be assigned to specific domain.
copy&paste error?
"assigned to specific domain" -> "unassign from specific domain"?
ok.
+ * Global counter will be freed once it is unassigned from all the domains.
Needs imperative tone.
+ */
+int rdtgroup_unassign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
+ struct rdt_mon_domain *d, enum resctrl_event_id evtid)
+{
+ int index = MBM_EVENT_ARRAY_INDEX(evtid);
+ int cntr_id = rdtgrp->mon.cntr_id[index];
+ int ret;
+
+ /* Return early if the counter is unassigned already */
+ if (cntr_id == MON_CNTR_UNSET)
+ return 0;
+
+ if (!d) {
+ list_for_each_entry(d, &r->mon_domains, hdr.list) {
+ ret = resctrl_arch_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
+ rdtgrp->closid, cntr_id, false);
+ if (ret)
+ goto out_done_unassign;
+
+ clear_bit(cntr_id, d->mbm_cntr_map);
+ }
+ } else {
+ ret = resctrl_arch_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
+ rdtgrp->closid, cntr_id, false);
+ if (ret)
+ goto out_done_unassign;
+
+ clear_bit(cntr_id, d->mbm_cntr_map);
Please see comment to previous patch about the duplicate snippets. Snippets can be
replaced with single function that also resets architectural state.
Sure.
will combine rdtgroup_assign_cntr_event() and
rdtgroup_unassign_cntr_event().
I need to rename the function. How about resctrl_configure_cntr_event()?
+ }
+
+ /* Update the counter bitmap */
What is the update?
Clear the counter bitmap.
+ if (!mbm_cntr_assigned_to_domain(r, cntr_id)) {
+ mbm_cntr_free(r, cntr_id);
+ rdtgrp->mon.cntr_id[index] = MON_CNTR_UNSET;
+ }
+
+out_done_unassign:
+ return ret;
+}
+
/* rdtgroup information files for one cache resource. */
static struct rftype res_common_files[] = {
{
Reinette
--
- Babu Moger
