linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features
@ 2023-02-02 21:46 Babu Moger
  2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:46 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

These series adds support few minor features.
1. Support assigning multiple tasks to control/mon groups in one command.
2. Add debug mount option for resctrl interface.
3. Add RMID and CLOSID in resctrl interface when mounted with debug option.
4. While doing these above changes, found that rftype flags needed some cleanup.
   They were named inconsistently. Re-arranged them much more cleanly now.
   Hope it can help future additions.

Code is built on top of tip branch x86/cache.

---
v2: Changes since v1
  a. Removed the changes to add the task's threads automatically. It required
     book keeping to handle the failures and gets complicated. Removed that change
     for now.
  b. Added -o debug option to mount in debug mode(comment from Fenghua)
  c. Added debug files rmid and closid. Stephane wanted to rename them more
     generic to accommodate ARM. It kind of loses meaning if is renamed differently.
     Kept it same for now. Will change if he feels strong about it. 

Babu Moger (7):
      x86/resctrl: Add multiple tasks to the resctrl group at once
      x86/resctrl: Remove few unnecessary rftype flags
      x86/resctrl: Rename rftype flags for consistency
      x86/resctrl: Re-arrange RFTYPE flags based on hierarchy
      x86/resctrl: Introduce -o debug mount option
      x86/resctrl: Display the RMID and COSID for resctrl groups
      x86/resctrl: Add debug files when mounted with debug option


 Documentation/x86/resctrl.rst          |  28 ++++-
 arch/x86/kernel/cpu/resctrl/core.c     |   8 +-
 arch/x86/kernel/cpu/resctrl/internal.h |  68 +++++++++--
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 154 +++++++++++++++++++++----
 4 files changed, 215 insertions(+), 43 deletions(-)

--


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
@ 2023-02-02 21:46 ` Babu Moger
  2023-02-16 22:27   ` Fenghua Yu
  2023-02-02 21:47 ` [RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags Babu Moger
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:46 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

The resctrl task assignment for MONITOR or CONTROL group needs to be
done one at a time. For example:

  $mount -t resctrl resctrl /sys/fs/resctrl/
  $mkdir /sys/fs/resctrl/clos1
  $echo 123 > /sys/fs/resctrl/clos1/tasks
  $echo 456 > /sys/fs/resctrl/clos1/tasks
  $echo 789 > /sys/fs/resctrl/clos1/tasks

This is not user-friendly when dealing with hundreds of tasks.

Improve the user experience by supporting the multiple task assignment
in one command with the tasks separated by commas. For example:

  $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/x86/resctrl.rst          |    9 +++++++--
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   24 +++++++++++++++++++++++-
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 058257dc56c8..58b76fc75cb7 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -292,13 +292,18 @@ All groups contain the following files:
 "tasks":
 	Reading this file shows the list of all tasks that belong to
 	this group. Writing a task id to the file will add a task to the
-	group. If the group is a CTRL_MON group the task is removed from
+	group. Multiple tasks can be assigned together in one command by
+	inputting the tasks separated by commas. Tasks will be assigned
+	sequentially in the order it is provided. Failure while assigning
+	the tasks will be aborted immediately and tasks next in the
+	sequence will not be assigned. Users may need to retry them again.
+
+	If the group is a CTRL_MON group the task is removed from
 	whichever previous CTRL_MON group owned the task and also from
 	any MON group that owned the task. If the group is a MON group,
 	then the task must already belong to the CTRL_MON parent of this
 	group. The task is removed from any previous MON group.
 
-
 "cpus":
 	Reading this file shows a bitmask of the logical CPUs owned by
 	this group. Writing a mask to this file will add and remove
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e2c1599d1b37..13b7c5f3a27c 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -683,16 +683,34 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
 				    char *buf, size_t nbytes, loff_t off)
 {
 	struct rdtgroup *rdtgrp;
+	char *pid_str;
 	int ret = 0;
 	pid_t pid;
 
-	if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
+	/* Valid input requires a trailing newline */
+	if (nbytes == 0 || buf[nbytes - 1] != '\n')
 		return -EINVAL;
+
+	buf[nbytes - 1] = '\0';
+
 	rdtgrp = rdtgroup_kn_lock_live(of->kn);
 	if (!rdtgrp) {
 		rdtgroup_kn_unlock(of->kn);
 		return -ENOENT;
 	}
+
+next:
+	if (!buf || buf[0] == '\0')
+		goto unlock;
+
+	pid_str = strim(strsep(&buf, ","));
+
+	if (kstrtoint(pid_str, 0, &pid) || pid < 0) {
+		rdt_last_cmd_puts("Invalid pid value\n");
+		ret = -EINVAL;
+		goto unlock;
+	}
+
 	rdt_last_cmd_clear();
 
 	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
@@ -703,6 +721,10 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
 	}
 
 	ret = rdtgroup_move_task(pid, rdtgrp, of);
+	if (ret)
+		goto unlock;
+	else
+		goto next;
 
 unlock:
 	rdtgroup_kn_unlock(of->kn);



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
  2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-02 21:47 ` [RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency Babu Moger
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

Remove few unnecessary rftype flags and simplify the code. This is done
to further cleanup the code eventually.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h |    9 +++------
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   10 +++++++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 8edecc5763d8..571145d75d29 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -243,12 +243,9 @@ struct rdtgroup {
  */
 #define RFTYPE_INFO			BIT(0)
 #define RFTYPE_BASE			BIT(1)
-#define RF_CTRLSHIFT			4
-#define RF_MONSHIFT			5
-#define RF_TOPSHIFT			6
-#define RFTYPE_CTRL			BIT(RF_CTRLSHIFT)
-#define RFTYPE_MON			BIT(RF_MONSHIFT)
-#define RFTYPE_TOP			BIT(RF_TOPSHIFT)
+#define RFTYPE_CTRL			BIT(4)
+#define RFTYPE_MON			BIT(5)
+#define RFTYPE_TOP			BIT(6)
 #define RFTYPE_RES_CACHE		BIT(8)
 #define RFTYPE_RES_MB			BIT(9)
 #define RF_CTRL_INFO			(RFTYPE_INFO | RFTYPE_CTRL)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 13b7c5f3a27c..cccf3fb84b26 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -3163,7 +3163,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 {
 	struct rdtgroup *prdtgrp, *rdtgrp;
 	struct kernfs_node *kn;
-	uint files = 0;
+	uint fflags = 0;
 	int ret;
 
 	prdtgrp = rdtgroup_kn_lock_live(parent_kn);
@@ -3215,8 +3215,12 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
 		goto out_destroy;
 	}
 
-	files = RFTYPE_BASE | BIT(RF_CTRLSHIFT + rtype);
-	ret = rdtgroup_add_files(kn, files);
+	if (rtype == RDTCTRL_GROUP)
+		fflags = RFTYPE_BASE | RFTYPE_CTRL;
+	else
+		fflags = RFTYPE_BASE | RFTYPE_MON;
+
+	ret = rdtgroup_add_files(kn, fflags);
 	if (ret) {
 		rdt_last_cmd_puts("kernfs fill error\n");
 		goto out_destroy;



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
  2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
  2023-02-02 21:47 ` [RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-02 21:47 ` [RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy Babu Moger
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

The rftype flags have two different prefixes even though they are used
for the same purpose. Change the prefix to RFTYPE_ for all the flags.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h |    8 +++---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   42 ++++++++++++++++----------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 571145d75d29..2cfc3c431d5b 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -248,10 +248,10 @@ struct rdtgroup {
 #define RFTYPE_TOP			BIT(6)
 #define RFTYPE_RES_CACHE		BIT(8)
 #define RFTYPE_RES_MB			BIT(9)
-#define RF_CTRL_INFO			(RFTYPE_INFO | RFTYPE_CTRL)
-#define RF_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
-#define RF_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
-#define RF_CTRL_BASE			(RFTYPE_BASE | RFTYPE_CTRL)
+#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
+#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
+#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
+#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
 
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index cccf3fb84b26..018a26b58154 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1691,77 +1691,77 @@ static struct rftype res_common_files[] = {
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_last_cmd_status_show,
-		.fflags		= RF_TOP_INFO,
+		.fflags		= RFTYPE_TOP_INFO,
 	},
 	{
 		.name		= "num_closids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_closids_show,
-		.fflags		= RF_CTRL_INFO,
+		.fflags		= RFTYPE_CTRL_INFO,
 	},
 	{
 		.name		= "mon_features",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_mon_features_show,
-		.fflags		= RF_MON_INFO,
+		.fflags		= RFTYPE_MON_INFO,
 	},
 	{
 		.name		= "num_rmids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_rmids_show,
-		.fflags		= RF_MON_INFO,
+		.fflags		= RFTYPE_MON_INFO,
 	},
 	{
 		.name		= "cbm_mask",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_default_ctrl_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "min_cbm_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_cbm_bits_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "shareable_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_shareable_bits_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "bit_usage",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bit_usage_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "min_bandwidth",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_bw_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	{
 		.name		= "bandwidth_gran",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bw_gran_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	{
 		.name		= "delay_linear",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_delay_linear_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	/*
 	 * Platform specific which (if any) capabilities are provided by
@@ -1780,7 +1780,7 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= max_threshold_occ_write,
 		.seq_show	= max_threshold_occ_show,
-		.fflags		= RF_MON_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_MON_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "mbm_total_bytes_config",
@@ -1827,7 +1827,7 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_schemata_write,
 		.seq_show	= rdtgroup_schemata_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 	{
 		.name		= "mode",
@@ -1835,14 +1835,14 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_mode_write,
 		.seq_show	= rdtgroup_mode_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 	{
 		.name		= "size",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdtgroup_size_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 
 };
@@ -1899,7 +1899,7 @@ void __init thread_throttle_mode_init(void)
 	if (!rft)
 		return;
 
-	rft->fflags = RF_CTRL_INFO | RFTYPE_RES_MB;
+	rft->fflags = RFTYPE_CTRL_INFO | RFTYPE_RES_MB;
 }
 
 void __init mbm_config_rftype_init(const char *config)
@@ -1908,7 +1908,7 @@ void __init mbm_config_rftype_init(const char *config)
 
 	rft = rdtgroup_get_rftype_by_name(config);
 	if (rft)
-		rft->fflags = RF_MON_INFO | RFTYPE_RES_CACHE;
+		rft->fflags = RFTYPE_MON_INFO | RFTYPE_RES_CACHE;
 }
 
 /**
@@ -2043,21 +2043,21 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 	if (IS_ERR(kn_info))
 		return PTR_ERR(kn_info);
 
-	ret = rdtgroup_add_files(kn_info, RF_TOP_INFO);
+	ret = rdtgroup_add_files(kn_info, RFTYPE_TOP_INFO);
 	if (ret)
 		goto out_destroy;
 
 	/* loop over enabled controls, these are all alloc_capable */
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		fflags =  r->fflags | RF_CTRL_INFO;
+		fflags =  r->fflags | RFTYPE_CTRL_INFO;
 		ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
 		if (ret)
 			goto out_destroy;
 	}
 
 	for_each_mon_capable_rdt_resource(r) {
-		fflags =  r->fflags | RF_MON_INFO;
+		fflags =  r->fflags | RFTYPE_MON_INFO;
 		sprintf(name, "%s_MON", r->name);
 		ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
 		if (ret)
@@ -3554,7 +3554,7 @@ static int __init rdtgroup_setup_root(void)
 
 	list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
 
-	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RF_CTRL_BASE);
+	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_CTRL_BASE);
 	if (ret) {
 		kernfs_destroy_root(rdt_root);
 		goto out;



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
                   ` (2 preceding siblings ...)
  2023-02-02 21:47 ` [RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-02 21:47 ` [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option Babu Moger
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

RESCTRL filesystem has two main components:
a. info (Details on resources and monitoring)
b. base (Details on CONTROL and MON groups)

The rftype flags can be renamed accordingly for better understanding.
For example:
RFTYPE_INFO     : Files with these flags go in info directory
RFTYPE_INFO_MON : Files with these flags go in info/L3_MON
RFTYPE_BASE     : Files with these flags go in group's(control or mon)
                  base directory
RFTYPE_BASE_CTRL: Files with these flags go in only CONTROL groups

Add comments to make it easy for future additions.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/core.c     |    8 ++--
 arch/x86/kernel/cpu/resctrl/internal.h |   64 ++++++++++++++++++++++++++++----
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   42 +++++++++++----------
 3 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index 030d3b409768..d1c6b2cc8611 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -69,7 +69,7 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.domains		= domain_init(RDT_RESOURCE_L3),
 			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
-			.fflags			= RFTYPE_RES_CACHE,
+			.fflags			= RFTYPE_CACHE,
 		},
 		.msr_base		= MSR_IA32_L3_CBM_BASE,
 		.msr_update		= cat_wrmsr,
@@ -83,7 +83,7 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.domains		= domain_init(RDT_RESOURCE_L2),
 			.parse_ctrlval		= parse_cbm,
 			.format_str		= "%d=%0*x",
-			.fflags			= RFTYPE_RES_CACHE,
+			.fflags			= RFTYPE_CACHE,
 		},
 		.msr_base		= MSR_IA32_L2_CBM_BASE,
 		.msr_update		= cat_wrmsr,
@@ -97,7 +97,7 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.domains		= domain_init(RDT_RESOURCE_MBA),
 			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
-			.fflags			= RFTYPE_RES_MB,
+			.fflags			= RFTYPE_MB,
 		},
 	},
 	[RDT_RESOURCE_SMBA] =
@@ -109,7 +109,7 @@ struct rdt_hw_resource rdt_resources_all[] = {
 			.domains		= domain_init(RDT_RESOURCE_SMBA),
 			.parse_ctrlval		= parse_bw,
 			.format_str		= "%d=%*u",
-			.fflags			= RFTYPE_RES_MB,
+			.fflags			= RFTYPE_MB,
 		},
 	},
 };
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 2cfc3c431d5b..6767c85b9699 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -240,18 +240,64 @@ struct rdtgroup {
 
 /*
  * Define the file type flags for base and info directories.
+ *
+ * RESCTRL filesystem has two main components
+ *   a. info
+ *   b. base.
+ *
+ * /sys/fs/resctrl/
+ *     |
+ *     --> info (Displays information about control and monitoring)
+ *     |
+ *     --> base (Displays the details on resctrl groups)
+ *
+ *    -------------------------------------------------------------
+ *     info directory structure
+ *     --> INFO
+ *         --> TOP
+ *             last_cmd_status
+ *
+ *         --> MON
+ *            --> (L2_MON)
+ *            --> (L3_MON)
+ *                max_threshold_occupancy, mbm_local_bytes_config,
+ *                mbm_total_bytes_config, mon_features, num_rmids
+ *
+ *         --> RES
+ *            --> CACHE (L2, L3)
+ *                bit_usage, cbm_mask, min_cbm_bits, num_closids,
+ *                shareable_bits
+ *
+ *            --> MB (MB, SMBA)
+ *                bandwidth_gran, delay_linear, min_bandwidth,
+ *                num_closids
+ *
+ *     group structure
+ *     -----------------------------------------------------------
+ *     --> BASE (Files common for both MON and CTRL groups)
+ *               cpus, cpus_list, tasks
+ *
+ *     --> CTRL (Files only for CTRL group)
+ *               mode, schemata, size
+ *
  */
 #define RFTYPE_INFO			BIT(0)
 #define RFTYPE_BASE			BIT(1)
-#define RFTYPE_CTRL			BIT(4)
-#define RFTYPE_MON			BIT(5)
-#define RFTYPE_TOP			BIT(6)
-#define RFTYPE_RES_CACHE		BIT(8)
-#define RFTYPE_RES_MB			BIT(9)
-#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
-#define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
-#define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
-#define RFTYPE_CTRL_BASE		(RFTYPE_BASE | RFTYPE_CTRL)
+
+#define RFTYPE_TOP			BIT(2)
+#define RFTYPE_MON			BIT(3)
+#define RFTYPE_RES			BIT(4)
+
+#define RFTYPE_CACHE			BIT(5)
+#define RFTYPE_MB			BIT(6)
+
+#define RFTYPE_CTRL			BIT(8)
+
+#define RFTYPE_INFO_TOP			(RFTYPE_INFO | RFTYPE_TOP)
+#define RFTYPE_INFO_MON			(RFTYPE_INFO | RFTYPE_MON)
+#define RFTYPE_INFO_RES			(RFTYPE_INFO | RFTYPE_RES)
+
+#define RFTYPE_BASE_CTRL		(RFTYPE_BASE | RFTYPE_CTRL)
 
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 018a26b58154..18d458a3cba6 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1691,77 +1691,77 @@ static struct rftype res_common_files[] = {
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_last_cmd_status_show,
-		.fflags		= RFTYPE_TOP_INFO,
+		.fflags		= RFTYPE_INFO_TOP,
 	},
 	{
 		.name		= "num_closids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_closids_show,
-		.fflags		= RFTYPE_CTRL_INFO,
+		.fflags		= RFTYPE_INFO_RES,
 	},
 	{
 		.name		= "mon_features",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_mon_features_show,
-		.fflags		= RFTYPE_MON_INFO,
+		.fflags		= RFTYPE_INFO_MON,
 	},
 	{
 		.name		= "num_rmids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_rmids_show,
-		.fflags		= RFTYPE_MON_INFO,
+		.fflags		= RFTYPE_INFO_MON,
 	},
 	{
 		.name		= "cbm_mask",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_default_ctrl_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_CACHE,
 	},
 	{
 		.name		= "min_cbm_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_cbm_bits_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_CACHE,
 	},
 	{
 		.name		= "shareable_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_shareable_bits_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_CACHE,
 	},
 	{
 		.name		= "bit_usage",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bit_usage_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_CACHE,
 	},
 	{
 		.name		= "min_bandwidth",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_bw_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_MB,
 	},
 	{
 		.name		= "bandwidth_gran",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bw_gran_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_MB,
 	},
 	{
 		.name		= "delay_linear",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_delay_linear_show,
-		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_INFO_RES | RFTYPE_MB,
 	},
 	/*
 	 * Platform specific which (if any) capabilities are provided by
@@ -1780,7 +1780,7 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= max_threshold_occ_write,
 		.seq_show	= max_threshold_occ_show,
-		.fflags		= RFTYPE_MON_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_INFO_MON | RFTYPE_CACHE,
 	},
 	{
 		.name		= "mbm_total_bytes_config",
@@ -1827,7 +1827,7 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_schemata_write,
 		.seq_show	= rdtgroup_schemata_show,
-		.fflags		= RFTYPE_CTRL_BASE,
+		.fflags		= RFTYPE_BASE_CTRL,
 	},
 	{
 		.name		= "mode",
@@ -1835,14 +1835,14 @@ static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_mode_write,
 		.seq_show	= rdtgroup_mode_show,
-		.fflags		= RFTYPE_CTRL_BASE,
+		.fflags		= RFTYPE_BASE_CTRL,
 	},
 	{
 		.name		= "size",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdtgroup_size_show,
-		.fflags		= RFTYPE_CTRL_BASE,
+		.fflags		= RFTYPE_BASE_CTRL,
 	},
 
 };
@@ -1899,7 +1899,7 @@ void __init thread_throttle_mode_init(void)
 	if (!rft)
 		return;
 
-	rft->fflags = RFTYPE_CTRL_INFO | RFTYPE_RES_MB;
+	rft->fflags = RFTYPE_INFO_RES | RFTYPE_MB;
 }
 
 void __init mbm_config_rftype_init(const char *config)
@@ -1908,7 +1908,7 @@ void __init mbm_config_rftype_init(const char *config)
 
 	rft = rdtgroup_get_rftype_by_name(config);
 	if (rft)
-		rft->fflags = RFTYPE_MON_INFO | RFTYPE_RES_CACHE;
+		rft->fflags = RFTYPE_INFO_MON | RFTYPE_CACHE;
 }
 
 /**
@@ -2043,21 +2043,21 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 	if (IS_ERR(kn_info))
 		return PTR_ERR(kn_info);
 
-	ret = rdtgroup_add_files(kn_info, RFTYPE_TOP_INFO);
+	ret = rdtgroup_add_files(kn_info, RFTYPE_INFO_TOP);
 	if (ret)
 		goto out_destroy;
 
 	/* loop over enabled controls, these are all alloc_capable */
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		fflags =  r->fflags | RFTYPE_CTRL_INFO;
+		fflags =  r->fflags | RFTYPE_INFO_RES;
 		ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
 		if (ret)
 			goto out_destroy;
 	}
 
 	for_each_mon_capable_rdt_resource(r) {
-		fflags =  r->fflags | RFTYPE_MON_INFO;
+		fflags =  r->fflags | RFTYPE_INFO_MON;
 		sprintf(name, "%s_MON", r->name);
 		ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
 		if (ret)
@@ -3554,7 +3554,7 @@ static int __init rdtgroup_setup_root(void)
 
 	list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
 
-	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_CTRL_BASE);
+	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_BASE_CTRL);
 	if (ret) {
 		kernfs_destroy_root(rdt_root);
 		goto out;



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
                   ` (3 preceding siblings ...)
  2023-02-02 21:47 ` [RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-17  1:42   ` Fenghua Yu
  2023-02-02 21:47 ` [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

Add -o debug option to mount resctrl filesystem in debug mode. Debug
option adds the files for debug purposes.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/x86/resctrl.rst          |    2 ++
 arch/x86/kernel/cpu/resctrl/internal.h |    1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |    7 +++++++
 3 files changed, 10 insertions(+)

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 58b76fc75cb7..2c013c5d45fd 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -46,6 +46,8 @@ mount options are:
 "mba_MBps":
 	Enable the MBA Software Controller(mba_sc) to specify MBA
 	bandwidth in MBps
+"debug":
+	Lists the debug files in resctrl interface
 
 L2 and L3 CDP are controlled separately.
 
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 6767c85b9699..35a9ee343fe0 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -59,6 +59,7 @@ struct rdt_fs_context {
 	bool				enable_cdpl2;
 	bool				enable_cdpl3;
 	bool				enable_mba_mbps;
+	bool				debug;
 };
 
 static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 18d458a3cba6..9b7813aa6baf 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2555,6 +2555,7 @@ enum rdt_param {
 	Opt_cdp,
 	Opt_cdpl2,
 	Opt_mba_mbps,
+	Opt_debug,
 	nr__rdt_params
 };
 
@@ -2562,6 +2563,7 @@ static const struct fs_parameter_spec rdt_fs_parameters[] = {
 	fsparam_flag("cdp",		Opt_cdp),
 	fsparam_flag("cdpl2",		Opt_cdpl2),
 	fsparam_flag("mba_MBps",	Opt_mba_mbps),
+	fsparam_flag("debug",		Opt_debug),
 	{}
 };
 
@@ -2587,6 +2589,9 @@ static int rdt_parse_param(struct fs_context *fc, struct fs_parameter *param)
 			return -EINVAL;
 		ctx->enable_mba_mbps = true;
 		return 0;
+	case Opt_debug:
+		ctx->debug = true;
+		return 0;
 	}
 
 	return -EINVAL;
@@ -3525,6 +3530,8 @@ static int rdtgroup_show_options(struct seq_file *seq, struct kernfs_root *kf)
 	if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl))
 		seq_puts(seq, ",mba_MBps");
 
+	seq_puts(seq, ",debug");
+
 	return 0;
 }
 



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
                   ` (4 preceding siblings ...)
  2023-02-02 21:47 ` [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-17  1:46   ` Fenghua Yu
  2023-02-02 21:47 ` [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option Babu Moger
  2023-02-16 18:05 ` [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Moger, Babu
  7 siblings, 1 reply; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

When a user creates a control or monitor group, the CLOSID or RMID
are not visible to the user. These are architecturally defined entities.
There is no harm in displaying these in resctrl groups. Sometimes it
can help to debug the issues.

Add CLOSID and RMID to the control/monitor groups display in resctrl
interface.

$cat /sys/fs/resctrl/clos1/closid
1
$cat /sys/fs/resctrl/mon_groups/mon1/rmid
3

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/x86/resctrl.rst          |   17 ++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   44 ++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 2c013c5d45fd..de332c242523 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -321,6 +321,15 @@ All groups contain the following files:
 	Just like "cpus", only using ranges of CPUs instead of bitmasks.
 
 
+"rmid":
+        Available only with debug option.Reading this file shows the
+        resource monitoring id (RMID) for monitoring the resource
+        utilization. Monitoring is performed by tagging each core(or
+        thread) or process via a Resource Monitoring ID (RMID). Kernel
+        assigns a new RMID when a group is created depending on the
+        available RMIDs. Multiple cores(or threads) or processes can
+        share a same RMID in a resctrl domain.
+
 When control is enabled all CTRL_MON groups will also contain:
 
 "schemata":
@@ -342,6 +351,14 @@ When control is enabled all CTRL_MON groups will also contain:
 	file. On successful pseudo-locked region creation the mode will
 	automatically change to "pseudo-locked".
 
+"closid":
+        Available only with debug option. Reading this file shows the
+        Class of Service (CLOS) id which acts as a resource control tag
+        on which the resources can be throttled. Kernel assigns a new
+        CLOSID a control group is created depending on the available
+        CLOSIDs. Multiple cores(or threads) or processes can share a
+        same CLOSID in a resctrl domain.
+
 When monitoring is enabled all MON groups will also contain:
 
 "mon_data":
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 9b7813aa6baf..c35d91b04de6 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -760,6 +760,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
 	return ret;
 }
 
+static int rdtgroup_closid_show(struct kernfs_open_file *of,
+				struct seq_file *s, void *v)
+{
+	struct rdtgroup *rdtgrp;
+	int ret = 0;
+
+	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+	if (rdtgrp)
+		seq_printf(s, "%u\n", rdtgrp->closid);
+	else
+		ret = -ENOENT;
+	rdtgroup_kn_unlock(of->kn);
+
+	return ret;
+}
+
+static int rdtgroup_rmid_show(struct kernfs_open_file *of,
+			      struct seq_file *s, void *v)
+{
+	struct rdtgroup *rdtgrp;
+	int ret = 0;
+
+	rdtgrp = rdtgroup_kn_lock_live(of->kn);
+	if (rdtgrp)
+		seq_printf(s, "%u\n", rdtgrp->mon.rmid);
+	else
+		ret = -ENOENT;
+	rdtgroup_kn_unlock(of->kn);
+
+	return ret;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*
@@ -1844,6 +1876,18 @@ static struct rftype res_common_files[] = {
 		.seq_show	= rdtgroup_size_show,
 		.fflags		= RFTYPE_BASE_CTRL,
 	},
+	{
+		.name		= "closid",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_closid_show,
+	},
+	{
+		.name		= "rmid",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_rmid_show,
+	},
 
 };
 



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
                   ` (5 preceding siblings ...)
  2023-02-02 21:47 ` [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger
@ 2023-02-02 21:47 ` Babu Moger
  2023-02-17  1:50   ` Fenghua Yu
  2023-02-16 18:05 ` [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Moger, Babu
  7 siblings, 1 reply; 18+ messages in thread
From: Babu Moger @ 2023-02-02 21:47 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: babu.moger, tglx, mingo, bp, dave.hansen, x86, hpa, corbet,
	linux-kernel, linux-doc, eranian, peternewman

Add the debug files to the resctrl hierarchy.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index c35d91b04de6..b7c72b011264 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2398,6 +2398,31 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
 			     struct rdtgroup *prgrp,
 			     struct kernfs_node **mon_data_kn);
 
+void resctrl_add_debug_file(struct kernfs_node *parent_kn,
+			    const char *config, unsigned long fflags,
+			    bool debug)
+{
+	struct rftype *rft;
+
+	rft = rdtgroup_get_rftype_by_name(config);
+	if (debug && rft) {
+		rft->fflags |= fflags;
+		rdtgroup_add_file(parent_kn, rft);
+	} else if (rft) {
+		rft->fflags &= ~fflags;
+		kernfs_remove_by_name(parent_kn, config);
+	}
+}
+
+static void resctrl_add_debug_files(bool debug)
+{
+	resctrl_add_debug_file(rdtgroup_default.kn, "rmid",
+			       RFTYPE_BASE, debug);
+	resctrl_add_debug_file(rdtgroup_default.kn, "closid",
+			       RFTYPE_BASE_CTRL, debug);
+	kernfs_activate(rdtgroup_default.kn);
+}
+
 static int rdt_enable_ctx(struct rdt_fs_context *ctx)
 {
 	int ret = 0;
@@ -2411,6 +2436,8 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx)
 	if (!ret && ctx->enable_mba_mbps)
 		ret = set_mba_sc(true);
 
+	resctrl_add_debug_files(ctx->debug);
+
 	return ret;
 }
 



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features
  2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
                   ` (6 preceding siblings ...)
  2023-02-02 21:47 ` [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option Babu Moger
@ 2023-02-16 18:05 ` Moger, Babu
  2023-02-16 22:13   ` Reinette Chatre
  7 siblings, 1 reply; 18+ messages in thread
From: Moger, Babu @ 2023-02-16 18:05 UTC (permalink / raw)
  To: fenghua.yu, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Gentle ping. Any comments on this!

Thanks

Babu

On 2/2/23 15:46, Babu Moger wrote:
> These series adds support few minor features.
> 1. Support assigning multiple tasks to control/mon groups in one command.
> 2. Add debug mount option for resctrl interface.
> 3. Add RMID and CLOSID in resctrl interface when mounted with debug option.
> 4. While doing these above changes, found that rftype flags needed some cleanup.
>    They were named inconsistently. Re-arranged them much more cleanly now.
>    Hope it can help future additions.
>
> Code is built on top of tip branch x86/cache.
>
> ---
> v2: Changes since v1
>   a. Removed the changes to add the task's threads automatically. It required
>      book keeping to handle the failures and gets complicated. Removed that change
>      for now.
>   b. Added -o debug option to mount in debug mode(comment from Fenghua)
>   c. Added debug files rmid and closid. Stephane wanted to rename them more
>      generic to accommodate ARM. It kind of loses meaning if is renamed differently.
>      Kept it same for now. Will change if he feels strong about it. 
>
> Babu Moger (7):
>       x86/resctrl: Add multiple tasks to the resctrl group at once
>       x86/resctrl: Remove few unnecessary rftype flags
>       x86/resctrl: Rename rftype flags for consistency
>       x86/resctrl: Re-arrange RFTYPE flags based on hierarchy
>       x86/resctrl: Introduce -o debug mount option
>       x86/resctrl: Display the RMID and COSID for resctrl groups
>       x86/resctrl: Add debug files when mounted with debug option
>
>
>  Documentation/x86/resctrl.rst          |  28 ++++-
>  arch/x86/kernel/cpu/resctrl/core.c     |   8 +-
>  arch/x86/kernel/cpu/resctrl/internal.h |  68 +++++++++--
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 154 +++++++++++++++++++++----
>  4 files changed, 215 insertions(+), 43 deletions(-)
>
> --
>
-- 
Thanks
Babu Moger


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features
  2023-02-16 18:05 ` [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Moger, Babu
@ 2023-02-16 22:13   ` Reinette Chatre
  0 siblings, 0 replies; 18+ messages in thread
From: Reinette Chatre @ 2023-02-16 22:13 UTC (permalink / raw)
  To: babu.moger, fenghua.yu
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi Babu,

On 2/16/2023 10:05 AM, Moger, Babu wrote:
> Gentle ping. Any comments on this!

It is in my queue. At the same time I hope that the
folks who pointed you into this direction would continue
to participate.

Reinette

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once
  2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
@ 2023-02-16 22:27   ` Fenghua Yu
  2023-02-17 15:05     ` Moger, Babu
  0 siblings, 1 reply; 18+ messages in thread
From: Fenghua Yu @ 2023-02-16 22:27 UTC (permalink / raw)
  To: Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi, Babu,

On 2/2/23 13:46, Babu Moger wrote:
> The resctrl task assignment for MONITOR or CONTROL group needs to be
> done one at a time. For example:
> 
>    $mount -t resctrl resctrl /sys/fs/resctrl/
>    $mkdir /sys/fs/resctrl/clos1
>    $echo 123 > /sys/fs/resctrl/clos1/tasks
>    $echo 456 > /sys/fs/resctrl/clos1/tasks
>    $echo 789 > /sys/fs/resctrl/clos1/tasks
> 
> This is not user-friendly when dealing with hundreds of tasks.

Maybe add something like "poor performance due to syscall overhead...".

> 
> Improve the user experience by supporting the multiple task assignment
> in one command with the tasks separated by commas. For example:
> 
>    $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   Documentation/x86/resctrl.rst          |    9 +++++++--
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   24 +++++++++++++++++++++++-
>   2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
> index 058257dc56c8..58b76fc75cb7 100644
> --- a/Documentation/x86/resctrl.rst
> +++ b/Documentation/x86/resctrl.rst
> @@ -292,13 +292,18 @@ All groups contain the following files:
>   "tasks":
>   	Reading this file shows the list of all tasks that belong to
>   	this group. Writing a task id to the file will add a task to the
> -	group. If the group is a CTRL_MON group the task is removed from
> +	group. Multiple tasks can be assigned together in one command by
> +	inputting the tasks separated by commas. Tasks will be assigned
> +	sequentially in the order it is provided. Failure while assigning
> +	the tasks will be aborted immediately and tasks next in the
> +	sequence will not be assigned. Users may need to retry them again.

May need to add "tasks before the failure are assigned...".

To retry movement, user needs to know which pid fails. So it's better
to add "last_command_status shows the failure pid and user can parse
it to retry assignment starting from the failure pid".

> +
> +	If the group is a CTRL_MON group the task is removed from
>   	whichever previous CTRL_MON group owned the task and also from
>   	any MON group that owned the task. If the group is a MON group,
>   	then the task must already belong to the CTRL_MON parent of this
>   	group. The task is removed from any previous MON group.
>   
> -
>   "cpus":
>   	Reading this file shows a bitmask of the logical CPUs owned by
>   	this group. Writing a mask to this file will add and remove
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index e2c1599d1b37..13b7c5f3a27c 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -683,16 +683,34 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
>   				    char *buf, size_t nbytes, loff_t off)
>   {
>   	struct rdtgroup *rdtgrp;
> +	char *pid_str;
>   	int ret = 0;
>   	pid_t pid;
>   
> -	if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
> +	/* Valid input requires a trailing newline */
> +	if (nbytes == 0 || buf[nbytes - 1] != '\n')
>   		return -EINVAL;
> +
> +	buf[nbytes - 1] = '\0';
> +
>   	rdtgrp = rdtgroup_kn_lock_live(of->kn);
>   	if (!rdtgrp) {
>   		rdtgroup_kn_unlock(of->kn);
>   		return -ENOENT;
>   	}
> +
> +next:
> +	if (!buf || buf[0] == '\0')
> +		goto unlock;
> +
> +	pid_str = strim(strsep(&buf, ","));
> +
> +	if (kstrtoint(pid_str, 0, &pid) || pid < 0) {
> +		rdt_last_cmd_puts("Invalid pid value\n");

Better to add pid_str in failure info. Then user knows where the failure 
pid happens and can re-do the movement starting from the failed pid.

> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
>   	rdt_last_cmd_clear();
>   
>   	if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
> @@ -703,6 +721,10 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
>   	}
>   
>   	ret = rdtgroup_move_task(pid, rdtgrp, of);
> +	if (ret)

May need to report "Failed at %d\n", pid;

> +		goto unlock;
> +	else
> +		goto next;
>   
>   unlock:
>   	rdtgroup_kn_unlock(of->kn);
> 
> 

Thanks.

-Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option
  2023-02-02 21:47 ` [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option Babu Moger
@ 2023-02-17  1:42   ` Fenghua Yu
  2023-02-17 17:29     ` Moger, Babu
  0 siblings, 1 reply; 18+ messages in thread
From: Fenghua Yu @ 2023-02-17  1:42 UTC (permalink / raw)
  To: Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi, Babu,

On 2/2/23 13:47, Babu Moger wrote:
> Add -o debug option to mount resctrl filesystem in debug mode. Debug
> option adds the files for debug purposes.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   Documentation/x86/resctrl.rst          |    2 ++
>   arch/x86/kernel/cpu/resctrl/internal.h |    1 +
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |    7 +++++++
>   3 files changed, 10 insertions(+)
> 
> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
> index 58b76fc75cb7..2c013c5d45fd 100644
> --- a/Documentation/x86/resctrl.rst
> +++ b/Documentation/x86/resctrl.rst
> @@ -46,6 +46,8 @@ mount options are:
>   "mba_MBps":
>   	Enable the MBA Software Controller(mba_sc) to specify MBA
>   	bandwidth in MBps
> +"debug":
> +	Lists the debug files in resctrl interface
>   
>   L2 and L3 CDP are controlled separately.
>   
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 6767c85b9699..35a9ee343fe0 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -59,6 +59,7 @@ struct rdt_fs_context {
>   	bool				enable_cdpl2;
>   	bool				enable_cdpl3;
>   	bool				enable_mba_mbps;
> +	bool				debug;
>   };
>   
>   static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 18d458a3cba6..9b7813aa6baf 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2555,6 +2555,7 @@ enum rdt_param {
>   	Opt_cdp,
>   	Opt_cdpl2,
>   	Opt_mba_mbps,
> +	Opt_debug,
>   	nr__rdt_params
>   };
>   
> @@ -2562,6 +2563,7 @@ static const struct fs_parameter_spec rdt_fs_parameters[] = {
>   	fsparam_flag("cdp",		Opt_cdp),
>   	fsparam_flag("cdpl2",		Opt_cdpl2),
>   	fsparam_flag("mba_MBps",	Opt_mba_mbps),
> +	fsparam_flag("debug",		Opt_debug),
>   	{}
>   };
>   
> @@ -2587,6 +2589,9 @@ static int rdt_parse_param(struct fs_context *fc, struct fs_parameter *param)
>   			return -EINVAL;
>   		ctx->enable_mba_mbps = true;
>   		return 0;
> +	case Opt_debug:
> +		ctx->debug = true;
> +		return 0;
>   	}
>   
>   	return -EINVAL;
> @@ -3525,6 +3530,8 @@ static int rdtgroup_show_options(struct seq_file *seq, struct kernfs_root *kf)
>   	if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl))
>   		seq_puts(seq, ",mba_MBps");
>   
> +	seq_puts(seq, ",debug");

Need to add a check here otherwise ",debug" will be always shown 
regardless "-o debug" is given or not:
+	if (ctx->debug)
+		seq_puts(seq, ",debug");

But I don't know a good way to get ctx->debug in this function yet. I 
think somehow ctx can be retrieved from kf but not sure.

> +
>   	return 0;
>   }
>   
> 
> 
Thanks.

-Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups
  2023-02-02 21:47 ` [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger
@ 2023-02-17  1:46   ` Fenghua Yu
  2023-02-17 17:39     ` Moger, Babu
  0 siblings, 1 reply; 18+ messages in thread
From: Fenghua Yu @ 2023-02-17  1:46 UTC (permalink / raw)
  To: Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi, Babu,

On 2/2/23 13:47, Babu Moger wrote:
> When a user creates a control or monitor group, the CLOSID or RMID
> are not visible to the user. These are architecturally defined entities.
> There is no harm in displaying these in resctrl groups. Sometimes it
> can help to debug the issues.
> 
> Add CLOSID and RMID to the control/monitor groups display in resctrl
> interface.
> 
> $cat /sys/fs/resctrl/clos1/closid
> 1
> $cat /sys/fs/resctrl/mon_groups/mon1/rmid
> 3
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   Documentation/x86/resctrl.rst          |   17 ++++++++++++
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   44 ++++++++++++++++++++++++++++++++
>   2 files changed, 61 insertions(+)
> 
> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
> index 2c013c5d45fd..de332c242523 100644
> --- a/Documentation/x86/resctrl.rst
> +++ b/Documentation/x86/resctrl.rst
> @@ -321,6 +321,15 @@ All groups contain the following files:
>   	Just like "cpus", only using ranges of CPUs instead of bitmasks.
>   
>   
> +"rmid":
> +        Available only with debug option.Reading this file shows the
> +        resource monitoring id (RMID) for monitoring the resource

Capitals Resource Monitoring ID (RMID).

> +        utilization. Monitoring is performed by tagging each core(or
> +        thread) or process via a Resource Monitoring ID (RMID). Kernel

s/Resource Monitoring ID (RMID)/RMID/

> +        assigns a new RMID when a group is created depending on the
> +        available RMIDs. Multiple cores(or threads) or processes can
> +        share a same RMID in a resctrl domain.
> +
>   When control is enabled all CTRL_MON groups will also contain:
>   
>   "schemata":
> @@ -342,6 +351,14 @@ When control is enabled all CTRL_MON groups will also contain:
>   	file. On successful pseudo-locked region creation the mode will
>   	automatically change to "pseudo-locked".
>   
> +"closid":
> +        Available only with debug option. Reading this file shows the
> +        Class of Service (CLOS) id which acts as a resource control tag
> +        on which the resources can be throttled. Kernel assigns a new
> +        CLOSID a control group is created depending on the available
> +        CLOSIDs. Multiple cores(or threads) or processes can share a
> +        same CLOSID in a resctrl domain.
> +
>   When monitoring is enabled all MON groups will also contain:
>   
>   "mon_data":
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 9b7813aa6baf..c35d91b04de6 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -760,6 +760,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
>   	return ret;
>   }
>   
> +static int rdtgroup_closid_show(struct kernfs_open_file *of,
> +				struct seq_file *s, void *v)
> +{
> +	struct rdtgroup *rdtgrp;
> +	int ret = 0;
> +
> +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> +	if (rdtgrp)
> +		seq_printf(s, "%u\n", rdtgrp->closid);
> +	else
> +		ret = -ENOENT;
> +	rdtgroup_kn_unlock(of->kn);
> +
> +	return ret;
> +}
> +
> +static int rdtgroup_rmid_show(struct kernfs_open_file *of,
> +			      struct seq_file *s, void *v)
> +{
> +	struct rdtgroup *rdtgrp;
> +	int ret = 0;
> +
> +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> +	if (rdtgrp)
> +		seq_printf(s, "%u\n", rdtgrp->mon.rmid);
> +	else
> +		ret = -ENOENT;
> +	rdtgroup_kn_unlock(of->kn);
> +
> +	return ret;
> +}
> +
>   #ifdef CONFIG_PROC_CPU_RESCTRL
>   
>   /*
> @@ -1844,6 +1876,18 @@ static struct rftype res_common_files[] = {
>   		.seq_show	= rdtgroup_size_show,
>   		.fflags		= RFTYPE_BASE_CTRL,
>   	},
> +	{
> +		.name		= "closid",
> +		.mode		= 0444,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= rdtgroup_closid_show,
> +	},
> +	{
> +		.name		= "rmid",
> +		.mode		= 0444,
> +		.kf_ops		= &rdtgroup_kf_single_ops,
> +		.seq_show	= rdtgroup_rmid_show,
> +	},
>   
>   };
>   
> 
> 
Thanks.

-Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option
  2023-02-02 21:47 ` [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option Babu Moger
@ 2023-02-17  1:50   ` Fenghua Yu
  2023-02-17 17:49     ` Moger, Babu
  0 siblings, 1 reply; 18+ messages in thread
From: Fenghua Yu @ 2023-02-17  1:50 UTC (permalink / raw)
  To: Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi, Babu,

On 2/2/23 13:47, Babu Moger wrote:
> Add the debug files to the resctrl hierarchy.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index c35d91b04de6..b7c72b011264 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2398,6 +2398,31 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>   			     struct rdtgroup *prgrp,
>   			     struct kernfs_node **mon_data_kn);
>   
> +void resctrl_add_debug_file(struct kernfs_node *parent_kn,
> +			    const char *config, unsigned long fflags,
> +			    bool debug)
> +{
> +	struct rftype *rft;
> +
> +	rft = rdtgroup_get_rftype_by_name(config);
> +	if (debug && rft) {
> +		rft->fflags |= fflags;
> +		rdtgroup_add_file(parent_kn, rft);
> +	} else if (rft) {
> +		rft->fflags &= ~fflags;
> +		kernfs_remove_by_name(parent_kn, config);
> +	}
> +}
> +
> +static void resctrl_add_debug_files(bool debug)
> +{
> +	resctrl_add_debug_file(rdtgroup_default.kn, "rmid",
> +			       RFTYPE_BASE, debug);
> +	resctrl_add_debug_file(rdtgroup_default.kn, "closid",
> +			       RFTYPE_BASE_CTRL, debug);
> +	kernfs_activate(rdtgroup_default.kn);
> +}
> +
>   static int rdt_enable_ctx(struct rdt_fs_context *ctx)
>   {
>   	int ret = 0;
> @@ -2411,6 +2436,8 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx)
>   	if (!ret && ctx->enable_mba_mbps)
>   		ret = set_mba_sc(true);
>    > +	resctrl_add_debug_files(ctx->debug);

It's better to change to:
+	if (ctx->debug)
+		resctrl_add_debug_files();

Then the functions in the call chain can remove 'debug' parameter and 
can be simpler.
> +
>   	return ret;
>   }
>   
> 
> 
Thanks.

-Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once
  2023-02-16 22:27   ` Fenghua Yu
@ 2023-02-17 15:05     ` Moger, Babu
  0 siblings, 0 replies; 18+ messages in thread
From: Moger, Babu @ 2023-02-17 15:05 UTC (permalink / raw)
  To: Fenghua Yu, Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi Fenghua,

On 2/16/2023 4:27 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:46, Babu Moger wrote:
>> The resctrl task assignment for MONITOR or CONTROL group needs to be
>> done one at a time. For example:
>>
>>    $mount -t resctrl resctrl /sys/fs/resctrl/
>>    $mkdir /sys/fs/resctrl/clos1
>>    $echo 123 > /sys/fs/resctrl/clos1/tasks
>>    $echo 456 > /sys/fs/resctrl/clos1/tasks
>>    $echo 789 > /sys/fs/resctrl/clos1/tasks
>>
>> This is not user-friendly when dealing with hundreds of tasks.
>
> Maybe add something like "poor performance due to syscall overhead...".
Ok. Sure
>
>>
>> Improve the user experience by supporting the multiple task assignment
>> in one command with the tasks separated by commas. For example:
>>
>>    $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   Documentation/x86/resctrl.rst          |    9 +++++++--
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   24 +++++++++++++++++++++++-
>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/x86/resctrl.rst 
>> b/Documentation/x86/resctrl.rst
>> index 058257dc56c8..58b76fc75cb7 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -292,13 +292,18 @@ All groups contain the following files:
>>   "tasks":
>>       Reading this file shows the list of all tasks that belong to
>>       this group. Writing a task id to the file will add a task to the
>> -    group. If the group is a CTRL_MON group the task is removed from
>> +    group. Multiple tasks can be assigned together in one command by
>> +    inputting the tasks separated by commas. Tasks will be assigned
>> +    sequentially in the order it is provided. Failure while assigning
>> +    the tasks will be aborted immediately and tasks next in the
>> +    sequence will not be assigned. Users may need to retry them again.
>
> May need to add "tasks before the failure are assigned...".
Sure.
>
> To retry movement, user needs to know which pid fails. So it's better
> to add "last_command_status shows the failure pid and user can parse
> it to retry assignment starting from the failure pid".
Sure.
>
>> +
>> +    If the group is a CTRL_MON group the task is removed from
>>       whichever previous CTRL_MON group owned the task and also from
>>       any MON group that owned the task. If the group is a MON group,
>>       then the task must already belong to the CTRL_MON parent of this
>>       group. The task is removed from any previous MON group.
>>   -
>>   "cpus":
>>       Reading this file shows a bitmask of the logical CPUs owned by
>>       this group. Writing a mask to this file will add and remove
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c 
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index e2c1599d1b37..13b7c5f3a27c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -683,16 +683,34 @@ static ssize_t rdtgroup_tasks_write(struct 
>> kernfs_open_file *of,
>>                       char *buf, size_t nbytes, loff_t off)
>>   {
>>       struct rdtgroup *rdtgrp;
>> +    char *pid_str;
>>       int ret = 0;
>>       pid_t pid;
>>   -    if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0)
>> +    /* Valid input requires a trailing newline */
>> +    if (nbytes == 0 || buf[nbytes - 1] != '\n')
>>           return -EINVAL;
>> +
>> +    buf[nbytes - 1] = '\0';
>> +
>>       rdtgrp = rdtgroup_kn_lock_live(of->kn);
>>       if (!rdtgrp) {
>>           rdtgroup_kn_unlock(of->kn);
>>           return -ENOENT;
>>       }
>> +
>> +next:
>> +    if (!buf || buf[0] == '\0')
>> +        goto unlock;
>> +
>> +    pid_str = strim(strsep(&buf, ","));
>> +
>> +    if (kstrtoint(pid_str, 0, &pid) || pid < 0) {
>> +        rdt_last_cmd_puts("Invalid pid value\n");
>
> Better to add pid_str in failure info. Then user knows where the 
> failure pid happens and can re-do the movement starting from the 
> failed pid.

ok.

>
>> +        ret = -EINVAL;
>> +        goto unlock;
>> +    }
>> +
>>       rdt_last_cmd_clear();
>>         if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
>> @@ -703,6 +721,10 @@ static ssize_t rdtgroup_tasks_write(struct 
>> kernfs_open_file *of,
>>       }
>>         ret = rdtgroup_move_task(pid, rdtgrp, of);
>> +    if (ret)
>
> May need to report "Failed at %d\n", pid;

Ok. Dont want to overwrite the last command status. I may need to update 
it with pid. Will check on this.

thanks

Babu

>
>> +        goto unlock;
>> +    else
>> +        goto next;
>>     unlock:
>>       rdtgroup_kn_unlock(of->kn);
>>
>>
>
> Thanks.
>
> -Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option
  2023-02-17  1:42   ` Fenghua Yu
@ 2023-02-17 17:29     ` Moger, Babu
  0 siblings, 0 replies; 18+ messages in thread
From: Moger, Babu @ 2023-02-17 17:29 UTC (permalink / raw)
  To: Fenghua Yu, Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi Fenghua,

On 2/16/2023 7:42 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:47, Babu Moger wrote:
>> Add -o debug option to mount resctrl filesystem in debug mode. Debug
>> option adds the files for debug purposes.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   Documentation/x86/resctrl.rst          |    2 ++
>>   arch/x86/kernel/cpu/resctrl/internal.h |    1 +
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |    7 +++++++
>>   3 files changed, 10 insertions(+)
>>
>> diff --git a/Documentation/x86/resctrl.rst 
>> b/Documentation/x86/resctrl.rst
>> index 58b76fc75cb7..2c013c5d45fd 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -46,6 +46,8 @@ mount options are:
>>   "mba_MBps":
>>       Enable the MBA Software Controller(mba_sc) to specify MBA
>>       bandwidth in MBps
>> +"debug":
>> +    Lists the debug files in resctrl interface
>>     L2 and L3 CDP are controlled separately.
>>   diff --git a/arch/x86/kernel/cpu/resctrl/internal.h 
>> b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 6767c85b9699..35a9ee343fe0 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -59,6 +59,7 @@ struct rdt_fs_context {
>>       bool                enable_cdpl2;
>>       bool                enable_cdpl3;
>>       bool                enable_mba_mbps;
>> +    bool                debug;
>>   };
>>     static inline struct rdt_fs_context *rdt_fc2context(struct 
>> fs_context *fc)
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c 
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 18d458a3cba6..9b7813aa6baf 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2555,6 +2555,7 @@ enum rdt_param {
>>       Opt_cdp,
>>       Opt_cdpl2,
>>       Opt_mba_mbps,
>> +    Opt_debug,
>>       nr__rdt_params
>>   };
>>   @@ -2562,6 +2563,7 @@ static const struct fs_parameter_spec 
>> rdt_fs_parameters[] = {
>>       fsparam_flag("cdp",        Opt_cdp),
>>       fsparam_flag("cdpl2",        Opt_cdpl2),
>>       fsparam_flag("mba_MBps",    Opt_mba_mbps),
>> +    fsparam_flag("debug",        Opt_debug),
>>       {}
>>   };
>>   @@ -2587,6 +2589,9 @@ static int rdt_parse_param(struct fs_context 
>> *fc, struct fs_parameter *param)
>>               return -EINVAL;
>>           ctx->enable_mba_mbps = true;
>>           return 0;
>> +    case Opt_debug:
>> +        ctx->debug = true;
>> +        return 0;
>>       }
>>         return -EINVAL;
>> @@ -3525,6 +3530,8 @@ static int rdtgroup_show_options(struct 
>> seq_file *seq, struct kernfs_root *kf)
>>       if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl))
>>           seq_puts(seq, ",mba_MBps");
>>   +    seq_puts(seq, ",debug");
>
> Need to add a check here otherwise ",debug" will be always shown 
> regardless "-o debug" is given or not:
> +    if (ctx->debug)
> +        seq_puts(seq, ",debug");
>
> But I don't know a good way to get ctx->debug in this function yet. I 
> think somehow ctx can be retrieved from kf but not sure.

Yes. Make sense. May be i will have to save it in rdt_hw_resource then I 
can add that check.

Thanks

Babu


>
>> +
>>       return 0;
>>   }
>>
>>
> Thanks.
>
> -Fenghua

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups
  2023-02-17  1:46   ` Fenghua Yu
@ 2023-02-17 17:39     ` Moger, Babu
  0 siblings, 0 replies; 18+ messages in thread
From: Moger, Babu @ 2023-02-17 17:39 UTC (permalink / raw)
  To: Fenghua Yu, Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman


On 2/16/2023 7:46 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:47, Babu Moger wrote:
>> When a user creates a control or monitor group, the CLOSID or RMID
>> are not visible to the user. These are architecturally defined entities.
>> There is no harm in displaying these in resctrl groups. Sometimes it
>> can help to debug the issues.
>>
>> Add CLOSID and RMID to the control/monitor groups display in resctrl
>> interface.
>>
>> $cat /sys/fs/resctrl/clos1/closid
>> 1
>> $cat /sys/fs/resctrl/mon_groups/mon1/rmid
>> 3
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   Documentation/x86/resctrl.rst          |   17 ++++++++++++
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   44 
>> ++++++++++++++++++++++++++++++++
>>   2 files changed, 61 insertions(+)
>>
>> diff --git a/Documentation/x86/resctrl.rst 
>> b/Documentation/x86/resctrl.rst
>> index 2c013c5d45fd..de332c242523 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -321,6 +321,15 @@ All groups contain the following files:
>>       Just like "cpus", only using ranges of CPUs instead of bitmasks.
>>     +"rmid":
>> +        Available only with debug option.Reading this file shows the
>> +        resource monitoring id (RMID) for monitoring the resource
>
> Capitals Resource Monitoring ID (RMID).
Sure.
>
>> +        utilization. Monitoring is performed by tagging each core(or
>> +        thread) or process via a Resource Monitoring ID (RMID). Kernel
>
> s/Resource Monitoring ID (RMID)/RMID/

Sure.

Thanks

Babu



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option
  2023-02-17  1:50   ` Fenghua Yu
@ 2023-02-17 17:49     ` Moger, Babu
  0 siblings, 0 replies; 18+ messages in thread
From: Moger, Babu @ 2023-02-17 17:49 UTC (permalink / raw)
  To: Fenghua Yu, Babu Moger, reinette.chatre
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel,
	linux-doc, eranian, peternewman

Hi Fenghua,

On 2/16/2023 7:50 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:47, Babu Moger wrote:
>> Add the debug files to the resctrl hierarchy.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   27 
>> +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c 
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index c35d91b04de6..b7c72b011264 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2398,6 +2398,31 @@ static int mkdir_mondata_all(struct 
>> kernfs_node *parent_kn,
>>                    struct rdtgroup *prgrp,
>>                    struct kernfs_node **mon_data_kn);
>>   +void resctrl_add_debug_file(struct kernfs_node *parent_kn,
>> +                const char *config, unsigned long fflags,
>> +                bool debug)
>> +{
>> +    struct rftype *rft;
>> +
>> +    rft = rdtgroup_get_rftype_by_name(config);
>> +    if (debug && rft) {
>> +        rft->fflags |= fflags;
>> +        rdtgroup_add_file(parent_kn, rft);
>> +    } else if (rft) {
>> +        rft->fflags &= ~fflags;
>> +        kernfs_remove_by_name(parent_kn, config);
>> +    }
>> +}
>> +
>> +static void resctrl_add_debug_files(bool debug)
>> +{
>> +    resctrl_add_debug_file(rdtgroup_default.kn, "rmid",
>> +                   RFTYPE_BASE, debug);
>> +    resctrl_add_debug_file(rdtgroup_default.kn, "closid",
>> +                   RFTYPE_BASE_CTRL, debug);
>> +    kernfs_activate(rdtgroup_default.kn);
>> +}
>> +
>>   static int rdt_enable_ctx(struct rdt_fs_context *ctx)
>>   {
>>       int ret = 0;
>> @@ -2411,6 +2436,8 @@ static int rdt_enable_ctx(struct rdt_fs_context 
>> *ctx)
>>       if (!ret && ctx->enable_mba_mbps)
>>           ret = set_mba_sc(true);
>>    > +    resctrl_add_debug_files(ctx->debug);
>
> It's better to change to:
> +    if (ctx->debug)
> +        resctrl_add_debug_files();
>
> Then the functions in the call chain can remove 'debug' parameter and 
> can be simpler.

Actually, debug parameter is required in the resctrl_add_debug_file to 
delete the file if it was mounted with debug option last time.

Thanks

Babu



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-02-17 17:49 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 21:46 [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Babu Moger
2023-02-02 21:46 ` [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger
2023-02-16 22:27   ` Fenghua Yu
2023-02-17 15:05     ` Moger, Babu
2023-02-02 21:47 ` [RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy Babu Moger
2023-02-02 21:47 ` [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option Babu Moger
2023-02-17  1:42   ` Fenghua Yu
2023-02-17 17:29     ` Moger, Babu
2023-02-02 21:47 ` [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger
2023-02-17  1:46   ` Fenghua Yu
2023-02-17 17:39     ` Moger, Babu
2023-02-02 21:47 ` [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option Babu Moger
2023-02-17  1:50   ` Fenghua Yu
2023-02-17 17:49     ` Moger, Babu
2023-02-16 18:05 ` [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features Moger, Babu
2023-02-16 22:13   ` Reinette Chatre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).