Hi Babu,
On 5/24/24 5:23 AM, Babu Moger wrote:
Hardware provides a set of counters when the ABMC feature is supported.
These counters are used for enabling the events in resctrl group when
the feature is supported.
Introduce num_cntrs_free_map bitmap to track available and free counters.
Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
v4: Changed the name to num_cntrs where applicable.
Used bitmap apis.
Added more comments for the globals.
v3: Changed the bitmap name to assign_cntrs_free_map. Removed abmc
from the name.
v2: Changed the bitmap name to assignable_counter_free_map from
abmc_counter_free_map.
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 3071bbb7a15e..400ae405e10e 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -186,6 +186,23 @@ bool closid_allocated(unsigned int closid)
return !test_bit(closid, &closid_free_map);
}
+/*
+ * ABMC Counter bitmap and length for tracking available counters.
+ * ABMC feature provides set of hardware counters for enabling events.
+ * Each event takes one hardware counter. Kernel needs to keep track
+ * of number of available counters.
+ */
+static unsigned long num_cntrs_free_map;
Why does variable have "num" in its name? That seems strange. How
about just "mon_cntrs_free_map
+static u32 num_cntrs_free_map_len;
Same comment about "num" ... also, any special reason why u32 is needed?
+
+static void num_cntrs_init(void)
mon_cntrs_init() ?
+{
+ struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+
+ bitmap_fill(&num_cntrs_free_map, r->mon.num_cntrs);
+ num_cntrs_free_map_len = r->mon.num_cntrs;
+}
+
/**
* rdtgroup_mode_by_closid - Return mode of resource group with closid
* @closid: closid if the resource group
@@ -2459,6 +2476,12 @@ static int resctrl_abmc_set_all(enum resctrl_res_level l, bool enable)
resctrl_abmc_set_all() was initially created as a complement of
resctrl_abmc_set_one() ... but with more initialization added to
resctrl_abmc_set_all() this relationship becomes vague.
struct rdt_resource *r = &rdt_resources_all[l].r_resctrl;
struct rdt_domain *d;
+ /*
+ * The bitmap num_cntrs_free_map needs to be reset when switching
+ * the monitor mode.
This comment puts in words what can be understood by just looking at the code.
What would be useful would be to explain _why_ this needs to be done ...
what happens to the previously assigned counters?
+ */
+ num_cntrs_init();
+
/*
* Update QOS_CFG MSR on all the CPUs associated with the resource
* Hardware counters will reset after switching the monotor mode.
Reinette
