Hi Reinette,
On 8/23/2024 3:18 PM, Reinette Chatre wrote:
Hi Babu,
On 8/21/24 9:01 AM, Moger, Babu wrote:
Hi Reinette,
On 8/16/24 16:41, Reinette Chatre wrote:
Hi Babu,
On 8/6/24 3:00 PM, Babu Moger wrote:
The ABMC feature provides an option to the user to assign a hardware
This is about resctrl fs so "The ABMC feature" -> "mbm_cntr_assign mode"
Sure.
(please check whole series).
Sure.
counter to an RMID and monitor the bandwidth as long as it is assigned.
The assigned RMID will be tracked by the hardware until the user
unassigns
it manually.
Hardware provides only limited number of counters. If the system
runs out
of assignable counters, kernel will display an error when a new
assignment
is requested. Users need to unassign a already assigned counter to make
space for new assignment.
Provide the interface to unassign the counter ids from the group.
Free the
counter if it is not assigned in any of the domains.
The feature details are documented in the APM listed below [1].
[1] AMD64 Architecture Programmer's Manual Volume 2: System Programming
Publication # 24593 Revision 3.41 section 19.3.3.3 Assignable
Bandwidth
Monitoring (ABMC).
Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
v6: Removed mbm_cntr_free from this patch.
Added counter test in all the domains and free if it is not
assigned to
any domains.
v5: Few name changes to match cntr_id.
Changed the function names to
rdtgroup_unassign_cntr
More comments on commit log.
v4: Added domain specific unassign feature.
Few name changes.
v3: Removed the static from the prototype of rdtgroup_unassign_abmc.
The function is not called directly from user anymore. These
changes are related to global assignment interface.
v2: No changes.
---
arch/x86/kernel/cpu/resctrl/internal.h | 2 +
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 52
++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
b/arch/x86/kernel/cpu/resctrl/internal.h
index 4e8109dee174..cc832955b787 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -689,6 +689,8 @@ int resctrl_arch_assign_cntr(struct rdt_mon_domain
*d, enum resctrl_event_id evt
u32 rmid, u32 cntr_id, u32 closid, bool assign);
int rdtgroup_assign_cntr(struct rdtgroup *rdtgrp, enum
resctrl_event_id evtid);
int rdtgroup_alloc_cntr(struct rdtgroup *rdtgrp, int index);
+int rdtgroup_unassign_cntr(struct rdtgroup *rdtgrp, enum
resctrl_event_id evtid);
+void rdtgroup_free_cntr(struct rdt_resource *r, struct rdtgroup
*rdtgrp, int index);
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 1ee91a7293a8..0c2215dbd497 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1961,6 +1961,58 @@ int rdtgroup_assign_cntr(struct rdtgroup
*rdtgrp,
enum resctrl_event_id evtid)
return 0;
}
+static int rdtgroup_mbm_cntr_test(struct rdt_resource *r, u32
cntr_id)
Could "test" be replaced with something more specific about what is
tested?
for example, "rdtgroup_mbm_cntr_is_assigned()" or something better? The
Yes. We can do that.
function
looks like a good candidate for returning a bool.
Sure.
Is this function needed though? (more below)
Yes. It is required. It is called from two places
(rdtgroup_unassign_update and rdtgroup_unassign_cntr).
We can open code in rdtgroup_unassign_cntr. But we can't do that in
rdtgroup_unassign_update. But, I will check again for sure.
Similar to rdtgroup_assign_cntr() and rdtgroup_assign_update() discussed
in previous patch, it also looks like rdtgroup_unassign_cntr() and
rdtgroup_unassign_update() can be merged.
Yes. We can do that. Thanks
--
- Babu Moger
