[RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features

[RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features

[Babu Moger]

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(-)

--

[RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once

[Babu Moger]

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);

Re: [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once

[Fenghua Yu]

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

Re: [RFC v2 PATCH 1/7] x86/resctrl: Add multiple tasks to the resctrl group at once

[Moger, Babu]

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

[RFC v2 PATCH 2/7] x86/resctrl: Remove few unnecessary rftype flags

[Babu Moger]

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;

[RFC v2 PATCH 3/7] x86/resctrl: Rename rftype flags for consistency

[Babu Moger]

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;

[RFC v2 PATCH 4/7] x86/resctrl: Re-arrange RFTYPE flags based on hierarchy

[Babu Moger]

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;

[RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option

[Babu Moger]

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;
 }
 

Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option

[Fenghua Yu]

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

Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option

[Moger, Babu]

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

[RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups

[Babu Moger]

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,
+	},
 
 };
 

Re: [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups

[Fenghua Yu]

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

Re: [RFC v2 PATCH 6/7] x86/resctrl: Display the RMID and COSID for resctrl groups

[Moger, Babu]

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

[RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option

[Babu Moger]

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;
 }
 

Re: [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option

[Fenghua Yu]

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

Re: [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with debug option

[Moger, Babu]

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

Re: [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features

[Moger, Babu]

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

Re: [RFC v2 PATCH 0/7] x86/resctrl: Miscellaneous resctrl features

[Reinette Chatre]

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