Hi Reinette,
On 9/19/2024 12:59 PM, Reinette Chatre wrote:
Hi Babu,
On 9/4/24 3:21 PM, Babu Moger wrote:
Introduce the interface to assign MBM events in mbm_cntr_assign mode.
Events can be enabled or disabled by writing to file
/sys/fs/resctrl/info/L3_MON/mbm_assign_control
Format is similar to the list format with addition of opcode for the
assignment operation.
"<CTRL_MON group>/<MON group>/<domain_id><opcode><flags>"
Format for specific type of groups:
* Default CTRL_MON group:
"//<domain_id><opcode><flags>"
* Non-default CTRL_MON group:
"<CTRL_MON group>//<domain_id><opcode><flags>"
* Child MON group of default CTRL_MON group:
"/<MON group>/<domain_id><opcode><flags>"
* Child MON group of non-default CTRL_MON group:
"<CTRL_MON group>/<MON group>/<domain_id><opcode><flags>"
Domain_id '*' will apply the flags on all the domains.
Opcode can be one of the following:
= Update the assignment to match the flags
+ Assign a new MBM event without impacting existing assignments.
- Unassign a MBM event from currently assigned events.
Assignment flags can be one of the following:
t MBM total event
l MBM local event
tl Both total and local MBM events
_ None of the MBM events. Valid only with '=' opcode.
Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
v7: Simplified the parsing (strsep(&token, "//") in rdtgroup_mbm_assign_control_write().
Added mutex lock in rdtgroup_mbm_assign_control_write() while processing.
Renamed rdtgroup_find_grp to rdtgroup_find_grp_by_name.
Fixed rdtgroup_str_to_mon_state to return error for invalid flags.
Simplified the calls rdtgroup_assign_cntr by merging few functions earlier.
Removed ABMC reference in FS code.
Reinette commented about handling the combination of flags like 'lt_' and '_lt'.
Not sure if we need to change the behaviour here. Processed them sequencially right now.
Users have the liberty to pass the flags. Restricting it might be a problem later.
Could you please give an example of what problem may be encountered later? An assignment
like "domain=_lt" seems like a contradiction to me since user space essentially asks
for "None of the MBM events" as well as "MBM total event" and "MBM local event".
I agree it is contradiction. But user is the one who decides to do that.
I think we should allow it. Also, there is some value to it as well.
"domain=_lt" This will also reset the counters if the total and local
events are assigned earlier this action.
...
@@ -352,6 +352,98 @@ with the following files:
There are four resctrl groups. All the groups have total and local MBM events
assigned on domain 0 and 1.
+ Assignment state can be updated by writing to the interface.
+
+ Format is similar to the list format with addition of opcode for the
+ assignment operation.
+
+ "<CTRL_MON group>/<MON group>/<domain_id><opcode><flags>"
+
+ Format for each type of groups:
+
+ * Default CTRL_MON group:
+ "//<domain_id><opcode><flags>"
+
+ * Non-default CTRL_MON group:
+ "<CTRL_MON group>//<domain_id><opcode><flags>"
+
+ * Child MON group of default CTRL_MON group:
+ "/<MON group>/<domain_id><opcode><flags>"
+
+ * Child MON group of non-default CTRL_MON group:
+ "<CTRL_MON group>/<MON group>/<domain_id><opcode><flags>"
+
+ Domain_id '*' will apply the flags on all the domains.
+
+ Opcode can be one of the following:
+ ::
+
+ = Update the assignment to match the MBM event.
+ + Assign a new MBM event without impacting existing assignments.
+ - Unassign a MBM event from currently assigned events.
+
+ Examples:
+ ::
+
+ Initial group status:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+ //0=tl;1=tl;
+ /child_default_mon_grp/0=tl;1=tl;
+
Similar to previous patch, looking at this generated doc does not seem to reflect
what is intended. Above and below are all formatted as code, the descriptions as
well as the actual "code".
Sure. Will check again.
+ To update the default group to assign only total MBM event on domain 0:
+ # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+ //0=t;1=tl;
+ /child_default_mon_grp/0=tl;1=tl;
+
+ To update the MON group child_default_mon_grp to remove total MBM event on domain 1:
+ # echo "/child_default_mon_grp/1-t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ $ cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+ //0=t;1=tl;
+ /child_default_mon_grp/0=tl;1=l;
+
+ To update the MON group non_default_ctrl_mon_grp/child_non_default_mon_grp to
+ unassign both local and total MBM events on domain 1:
+ # echo "non_default_ctrl_mon_grp/child_non_default_mon_grp/1=_" >
+ /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=_;
+ //0=t;1=tl;
+ /child_default_mon_grp/0=tl;1=l;
+
+ To update the default group to add a local MBM event domain 0.
+ # echo "//0+l" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=_;
+ //0=tl;1=tl;
+ /child_default_mon_grp/0=tl;1=l;
+
+ To update the non default CTRL_MON group non_default_ctrl_mon_grp to unassign all
+ the MBM events on all the domains.
+ # echo "non_default_ctrl_mon_grp//*=_" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ #cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ non_default_ctrl_mon_grp//0=_;1=_;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=_;
+ //0=tl;1=tl;
+ /child_default_mon_grp/0=tl;1=l;
+
"max_threshold_occupancy":
Read/write file provides the largest value (in
bytes) at which a previously used LLC_occupancy
...
+static int rdtgroup_process_flags(struct rdt_resource *r,
+ enum rdt_group_type rtype,
+ char *p_grp, char *c_grp, char *tok)
+{
+ int op, mon_state, assign_state, unassign_state;
+ char *dom_str, *id_str, *op_str;
+ struct rdt_mon_domain *d;
+ struct rdtgroup *rdtgrp;
+ unsigned long dom_id;
+ int ret, found = 0;
+
+ rdtgrp = rdtgroup_find_grp_by_name(rtype, p_grp, c_grp);
+
+ if (!rdtgrp) {
+ rdt_last_cmd_puts("Not a valid resctrl group\n");
+ return -EINVAL;
+ }
+
+next:
+ if (!tok || tok[0] == '\0')
+ return 0;
+
+ /* Start processing the strings for each domain */
+ dom_str = strim(strsep(&tok, ";"));
+
+ op_str = strpbrk(dom_str, "=+-");
+
+ if (op_str) {
+ op = *op_str;
+ } else {
+ rdt_last_cmd_puts("Missing operation =, +, -, _ character\n");
"_" is not an operation.
Sure. Will remove this charactor.
+ return -EINVAL;
+ }
+
+ id_str = strsep(&dom_str, "=+-");
+
+ /* Check for domain id '*' which means all domains */
+ if (id_str && *id_str == '*') {
+ d = NULL;
+ goto check_state;
+ } else if (!id_str || kstrtoul(id_str, 10, &dom_id)) {
+ rdt_last_cmd_puts("Missing domain id\n");
+ return -EINVAL;
+ }
+
+ /* Verify if the dom_id is valid */
+ list_for_each_entry(d, &r->mon_domains, hdr.list) {
+ if (d->hdr.id == dom_id) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+ rdt_last_cmd_printf("Invalid domain id %ld\n", dom_id);
+ return -EINVAL;
+ }
+
+check_state:
+ mon_state = rdtgroup_str_to_mon_state(dom_str);
+
+ if (mon_state == ASSIGN_INVALID) {
+ rdt_last_cmd_puts("Invalid assign flag\n");
+ goto out_fail;
+ }
+
+ assign_state = 0;
+ unassign_state = 0;
+
+ switch (op) {
+ case '+':
+ if (mon_state == ASSIGN_NONE) {
+ rdt_last_cmd_puts("Invalid assign opcode\n");
+ goto out_fail;
+ }
+ assign_state = mon_state;
+ break;
+ case '-':
+ if (mon_state == ASSIGN_NONE) {
+ rdt_last_cmd_puts("Invalid assign opcode\n");
+ goto out_fail;
+ }
+ unassign_state = mon_state;
+ break;
+ case '=':
+ assign_state = mon_state;
+ unassign_state = (ASSIGN_TOTAL | ASSIGN_LOCAL) & ~assign_state;
+ break;
+ default:
+ break;
+ }
+
+ if (assign_state & ASSIGN_TOTAL) {
+ ret = rdtgroup_assign_cntr(r, rdtgrp, d, QOS_L3_MBM_TOTAL_EVENT_ID);
hmmm ... wasn't unassign going to happen first? That would potentially make counters
available to help subsequent assign succeed.
Good point. I will change the order.
+ if (ret)
+ goto out_fail;
+ }
+
+ if (assign_state & ASSIGN_LOCAL) {
+ ret = rdtgroup_assign_cntr(r, rdtgrp, d, QOS_L3_MBM_LOCAL_EVENT_ID);
+ if (ret)
+ goto out_fail;
+ }
+
+ if (unassign_state & ASSIGN_TOTAL) {
+ ret = rdtgroup_unassign_cntr(r, rdtgrp, d, QOS_L3_MBM_TOTAL_EVENT_ID);
+ if (ret)
+ goto out_fail;
+ }
+
+ if (unassign_state & ASSIGN_LOCAL) {
+ ret = rdtgroup_unassign_cntr(r, rdtgrp, d, QOS_L3_MBM_LOCAL_EVENT_ID);
+ if (ret)
+ goto out_fail;
+ }
+
+ goto next;
+
+out_fail:
+
+ return -EINVAL;
+}
+
Reinette
Thanks
--
- Babu Moger
