Re: [PATCH v5 19/20] x86/resctrl: Introduce interface to list monitor states of all the groups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Babu,

On 7/3/24 2:48 PM, Babu Moger wrote:
Provide the interface to list the monitor states of all the resctrl
groups in ABMC mode.

Example:
$cat /sys/fs/resctrl/info/L3_MON/mbm_control

List follows the following format:

"<CTRL_MON group>/<MON group>/<domain_id>=<flags>"

Format for specific type of groups:

- Default CTRL_MON group:
   "//<domain_id>=<flags>"

- Non-default CTRL_MON group:
   "<CTRL_MON group>//<domain_id>=<flags>"

- Child MON group of default CTRL_MON group:
   "/<MON group>/<domain_id>=<flags>"

- Child MON group of non-default CTRL_MON group:
   "<CTRL_MON group>/<MON group>/<domain_id>=<flags>"


Flags can be one of the following:
t  MBM total event is enabled
l  MBM local event is enabled
tl Both total and local MBM events are enabled
_  None of the MBM events are enabled

Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
v5: Replaced "assignment flags" with "flags".
     Changes related to mon structure.
     Changes related renaming the interface from mbm_assign_control to
     mbm_control.

v4: Added functionality to query domain specific assigment in.
     rdtgroup_abmc_dom_state().

v3: New patch.
     Addresses the feedback to provide the global assignment interface.
     https://lore.kernel.org/lkml/c73f444b-83a1-4e9a-95d3-54c5165ee782@xxxxxxxxx/
---
  Documentation/arch/x86/resctrl.rst     |  54 ++++++++++
  arch/x86/kernel/cpu/resctrl/monitor.c  |   1 +
  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 130 +++++++++++++++++++++++++
  3 files changed, 185 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 4c41c5622627..05fee779e109 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -304,6 +304,60 @@ with the following files:
  "num_mbm_cntrs":
  	The number of monitoring counters available for assignment.
+"mbm_control":
+	Available when ABMC features are supported.

"Available when ABMC features are supported." can be dropped

+	Reports the resctrl group and monitor status of each group.
+
+	List follows the following format:
+		"<CTRL_MON group>/<MON group>/<domain_id>=<flags>"
+
+	Format for specific type of grpups:

grpups -> groups

+
+	* Default CTRL_MON group:
+		"//<domain_id>=<flags>"
+
+	* Non-default CTRL_MON group:
+		"<CTRL_MON group>//<domain_id>=<flags>"
+
+	* Child MON group of default CTRL_MON group:
+		"/<MON group>/<domain_id>=<flags>"
+
+	* Child MON group of non-default CTRL_MON group:
+		"<CTRL_MON group>/<MON group>/<domain_id>=<flags>"
+
+	Flags can be one of the following:
+	::
+
+	 t  MBM total event is enabled.
+	 l  MBM local event is enabled.
+	 tl Both total and local MBM events are enabled.
+	 _  None of the MBM events are enabled.
+
+	Examples:
+	::
+
+	 # mkdir /sys/fs/resctrl/mon_groups/child_default_mon_grp
+	 # mkdir /sys/fs/resctrl/non_default_ctrl_mon_grp
+	 # mkdir /sys/fs/resctrl/non_default_ctrl_mon_grp/mon_groups/child_non_default_mon_grp
+
+	 # cat /sys/fs/resctrl/info/L3_MON/mbm_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;
+
+	 There are four resctrl groups. All the groups have total and local events are
+	 enabled on domain 0 and 1.

"All the groups have total and local events are enabled" -> "All the groups have total and local events enabled"?

+

The text below seems to repeat ealier description.

+	 non_default_ctrl_mon_grp// - This is a non-default CTRL_MON group.
+
+	 non_default_ctrl_mon_grp/child_non_default_mon_grp/ - This is a child monitor
+	 group of non-default CTRL_MON group.
+
+	 // - This is a default CTRL_MON group.
+
+	 /child_default_mon_grp/ - This is a child monitor group of default CTRL_MON group.
+
  "max_threshold_occupancy":
  		Read/write file provides the largest value (in
  		bytes) at which a previously used LLC_occupancy
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index b96b0a8bd7d3..684730f1a72d 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -1244,6 +1244,7 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
  				r->mon.num_mbm_cntrs = 64;
resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
+			resctrl_file_fflags_init("mbm_control", RFTYPE_MON_INFO);

Shouldn't this file always be present?

  		}
  	}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d978668c8865..0de9f23d5389 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -944,6 +944,130 @@ static ssize_t rdtgroup_mbm_mode_write(struct kernfs_open_file *of,
  	return ret ?: nbytes;
  }
+static void rdtgroup_abmc_dom_cfg(void *info)
+{
+	u64 *msrval = info;
+
+	wrmsrl(MSR_IA32_L3_QOS_ABMC_CFG, *msrval);
+	rdmsrl(MSR_IA32_L3_QOS_ABMC_DSC, *msrval);
+}
+
+/*
+ * Writing the counter id with CfgEn=0 on L3_QOS_ABMC_CFG and reading
+ * L3_QOS_ABMC_DSC back will return configuration of the counter
+ * specified.

Can this be expanded to explain what the return values mean?

+ */
+static int rdtgroup_abmc_dom_state(struct rdt_mon_domain *d, u32 cntr_id,
+				   u32 rmid)
+{
+	union l3_qos_abmc_cfg abmc_cfg = { 0 };
+
+	abmc_cfg.split.cfg_en = 0;
+	abmc_cfg.split.cntr_id = cntr_id;
+
+	smp_call_function_any(&d->hdr.cpu_mask, rdtgroup_abmc_dom_cfg,
+			      &abmc_cfg, 1);
+
+	if (abmc_cfg.split.cntr_en && abmc_cfg.split.bw_src == rmid)
+		return 0;
+	else
+		return -1;
+}
+
+static char *rdtgroup_mon_state_to_str(struct rdtgroup *rdtgrp,
+				       struct rdt_mon_domain *d, char *str)
+{
+	char *tmp = str;
+	int dom_state = ASSIGN_NONE;

reverse fir

+
+	/*
+	 * Query the monitor state for the domain.
+	 * Index 0 for evtid == QOS_L3_MBM_TOTAL_EVENT_ID
+	 * Index 1 for evtid == QOS_L3_MBM_LOCAL_EVENT_ID

Why not use the helper?

+	 */
+	if (rdtgrp->mon.cntr_id[0] != MON_CNTR_UNSET)
+		if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[0], rdtgrp->mon.rmid))
+			dom_state |= ASSIGN_TOTAL;
+
+	if (rdtgrp->mon.cntr_id[1] != MON_CNTR_UNSET)
+		if (!rdtgroup_abmc_dom_state(d, rdtgrp->mon.cntr_id[1], rdtgrp->mon.rmid))
+			dom_state |= ASSIGN_LOCAL;
+
+	switch (dom_state) {
+	case ASSIGN_NONE:
+		*tmp++ = '_';
+		break;
+	case (ASSIGN_TOTAL | ASSIGN_LOCAL):
+		*tmp++ = 't';
+		*tmp++ = 'l';
+		break;
+	case ASSIGN_TOTAL:
+		*tmp++ = 't';
+		break;
+	case ASSIGN_LOCAL:
+		*tmp++ = 'l';
+		break;
+	default:
+		break;
+	}

This switch statement does not scale. Adding new flags will be painful. Can flags not
just incrementally be printed as learned from hardware with "_" printed as last resort?
This would elimininate need for these "ASSIGN" flags.

+
+	*tmp = '\0';
+	return str;
+}
+
+static int rdtgroup_mbm_control_show(struct kernfs_open_file *of,
+				     struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+	struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+	struct rdt_mon_domain *dom;
+	struct rdtgroup *rdtg;
+	int grp_default = 0;
+	char str[10];
+
+	if (!hw_res->abmc_enabled) {
+		rdt_last_cmd_puts("ABMC feature is not enabled\n");
+		return -EINVAL;
+	}
+
+	mutex_lock(&rdtgroup_mutex);
+
+	list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
+		struct rdtgroup *crg;
+
+		if (rdtg == &rdtgroup_default) {
+			grp_default = 1;
+			seq_puts(s, "//");
+		} else {
+			grp_default = 0;
+			seq_printf(s, "%s//", rdtg->kn->name);
+		}

Isn't the default resource group's name already empty string? That should
eliminate the need for this special handling, no?

+
+		list_for_each_entry(dom, &r->mon_domains, hdr.list)
+			seq_printf(s, "%d=%s;", dom->hdr.id,
+				   rdtgroup_mon_state_to_str(rdtg, dom, str));
+		seq_putc(s, '\n');
+
+		list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
+				    mon.crdtgrp_list) {
+			if (grp_default)
+				seq_printf(s, "/%s/", crg->kn->name);
+			else
+				seq_printf(s, "%s/%s/", rdtg->kn->name,
+					   crg->kn->name);
+

Same here .... with default group having name of empty string it can just be
printed directly, no?

+			list_for_each_entry(dom, &r->mon_domains, hdr.list)
+				seq_printf(s, "%d=%s;", dom->hdr.id,
+					   rdtgroup_mon_state_to_str(crg, dom, str));
+			seq_putc(s, '\n');
+		}
+	}
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
  #ifdef CONFIG_PROC_CPU_RESCTRL
/*
@@ -2156,6 +2280,12 @@ static struct rftype res_common_files[] = {
  		.kf_ops		= &rdtgroup_kf_single_ops,
  		.seq_show	= rdtgroup_num_mbm_cntrs_show,
  	},
+	{
+		.name		= "mbm_control",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_mbm_control_show,
+	},
  	{
  		.name		= "cpus_list",
  		.mode		= 0644,

Reinette




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux