[PATCH v4 03/31] fs/resctrl: Clean up rdtgroup_mba_mbps_event_{show,write}()

Tony Luck posted 31 patches 7 months, 3 weeks ago
There is a newer version of this series
[PATCH v4 03/31] fs/resctrl: Clean up rdtgroup_mba_mbps_event_{show,write}()
Posted by Tony Luck 7 months, 3 weeks ago
These routines hard-code the two legacy mbm events.

Change to allow for other mbm events in the future.

Signed-off-by: Tony Luck <tony.luck@intel.com>
---
 fs/resctrl/internal.h    |  4 ++++
 fs/resctrl/ctrlmondata.c | 39 +++++++++------------------------------
 fs/resctrl/monitor.c     | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
index ff89a0ca130e..6029b3285dd3 100644
--- a/fs/resctrl/internal.h
+++ b/fs/resctrl/internal.h
@@ -393,6 +393,10 @@ bool closid_allocated(unsigned int closid);
 
 int resctrl_find_cleanest_closid(void);
 
+enum resctrl_event_id resctrl_get_mon_event_by_name(char *name);
+
+char *resctrl_mon_event_name(enum resctrl_event_id evt);
+
 #ifdef CONFIG_RESCTRL_FS_PSEUDO_LOCK
 int rdtgroup_locksetup_enter(struct rdtgroup *rdtgrp);
 
diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
index b17b60114afd..53388281ff7d 100644
--- a/fs/resctrl/ctrlmondata.c
+++ b/fs/resctrl/ctrlmondata.c
@@ -472,26 +472,17 @@ ssize_t rdtgroup_mba_mbps_event_write(struct kernfs_open_file *of,
 	}
 	rdt_last_cmd_clear();
 
-	if (!strcmp(buf, "mbm_local_bytes")) {
-		if (resctrl_is_mon_event_enabled(QOS_L3_MBM_LOCAL_EVENT_ID))
-			rdtgrp->mba_mbps_event = QOS_L3_MBM_LOCAL_EVENT_ID;
-		else
-			ret = -EINVAL;
-	} else if (!strcmp(buf, "mbm_total_bytes")) {
-		if (resctrl_is_mon_event_enabled(QOS_L3_MBM_TOTAL_EVENT_ID))
-			rdtgrp->mba_mbps_event = QOS_L3_MBM_TOTAL_EVENT_ID;
-		else
-			ret = -EINVAL;
-	} else {
+	ret = resctrl_get_mon_event_by_name(buf);
+	if (ret < 0 || !resctrl_is_mon_event_enabled(ret) || !resctrl_is_mbm_event(ret)) {
+		rdt_last_cmd_printf("Unsupported event id '%s'\n", buf);
 		ret = -EINVAL;
+	} else {
+		rdtgrp->mba_mbps_event = ret;
 	}
 
-	if (ret)
-		rdt_last_cmd_printf("Unsupported event id '%s'\n", buf);
-
 	rdtgroup_kn_unlock(of->kn);
 
-	return ret ?: nbytes;
+	return ret < 0 ? ret : nbytes;
 }
 
 int rdtgroup_mba_mbps_event_show(struct kernfs_open_file *of,
@@ -502,22 +493,10 @@ int rdtgroup_mba_mbps_event_show(struct kernfs_open_file *of,
 
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 
-	if (rdtgrp) {
-		switch (rdtgrp->mba_mbps_event) {
-		case QOS_L3_MBM_LOCAL_EVENT_ID:
-			seq_puts(s, "mbm_local_bytes\n");
-			break;
-		case QOS_L3_MBM_TOTAL_EVENT_ID:
-			seq_puts(s, "mbm_total_bytes\n");
-			break;
-		default:
-			pr_warn_once("Bad event %d\n", rdtgrp->mba_mbps_event);
-			ret = -EINVAL;
-			break;
-		}
-	} else {
+	if (rdtgrp)
+		seq_printf(s, "%s\n", resctrl_mon_event_name(rdtgrp->mba_mbps_event));
+	else
 		ret = -ENOENT;
-	}
 
 	rdtgroup_kn_unlock(of->kn);
 
diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
index ef33970166af..625cd328c790 100644
--- a/fs/resctrl/monitor.c
+++ b/fs/resctrl/monitor.c
@@ -869,6 +869,22 @@ bool resctrl_is_mon_event_enabled(enum resctrl_event_id evtid)
 	return evtid < QOS_NUM_EVENTS && mon_event_all[evtid].enabled;
 }
 
+enum resctrl_event_id resctrl_get_mon_event_by_name(char *name)
+{
+	enum resctrl_event_id evt;
+
+	for (evt = 0; evt < QOS_NUM_EVENTS; evt++)
+		if (mon_event_all[evt].name && !strcmp(name, mon_event_all[evt].name))
+			return evt;
+
+	return -EINVAL;
+}
+
+char *resctrl_mon_event_name(enum resctrl_event_id evt)
+{
+	return evt < QOS_NUM_EVENTS && mon_event_all[evt].name ? mon_event_all[evt].name : "unknown";
+}
+
 /*
  * Initialize the event list for the resource.
  *
-- 
2.48.1
Re: [PATCH v4 03/31] fs/resctrl: Clean up rdtgroup_mba_mbps_event_{show,write}()
Posted by Reinette Chatre 7 months, 2 weeks ago
Hi Tony,

On 4/28/25 5:33 PM, Tony Luck wrote:
> These routines hard-code the two legacy mbm events.

No context, just a cryptic problem description.

> 
> Change to allow for other mbm events in the future.

What is changed? How are other MBM events allowed in future?

Something to start with (incomplete and needs improvement still):

	rdtgroup_mba_mbps_event_{show,write}() respectively shows and stores            
	which memory bandwidth event is used as input to the software feedback
	loop that keeps memory bandwidth below the value specified in the
	schemata file.

	rdtgroup_mba_mbps_event_{show,write}() hard codes the two legacy MBM            
	events, MBM total bytes and MBM local bytes. 

	(Needs explanation why the hardcoding is a problem and how this is fixed.
	 Since this series is not adding any new MBM events it is unclear what
  	 motivates this "in the future" fix as part of this telemetry work.)

Reinette