In x86, hardware uses CLOSID to identify a control group. When a user
creates a control group this information is not visible to the user.
It can help resctrl debugging.
Add CLOSID(ctrl_hw_id) to the control groups display in resctrl
interface. Users can see this detail when resctrl is mounted with
"-o debug" option.
Other architectures do not use "CLOSID". Use the names ctrl_hw_id
to refer to "CLOSID" in an effort to keep the naming generic.
For example:
$cat /sys/fs/resctrl/ctrl_grp1/ctrl_hw_id
1
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
Documentation/arch/x86/resctrl.rst | 4 ++++
arch/x86/kernel/cpu/resctrl/internal.h | 3 +++
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
3 files changed, 30 insertions(+)
diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 28d35aaa74b4..54691c8b832d 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -352,6 +352,10 @@ 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".
+"ctrl_hw_id":
+ Available only with debug option. The identifier used by hardware
+ for the control group. On x86 this is the CLOSID.
+
When monitoring is enabled all MON groups will also contain:
"mon_data":
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 38b99a21ccd8..ccdbed615d41 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -299,6 +299,9 @@ struct rdtgroup {
* --> RFTYPE_CTRL (Files only for CTRL group)
* Files: mode, schemata, size
*
+ * --> RFTYPE_DEBUG (Files to help resctrl debugging)
+ * File: ctrl_hw_id
+ *
*/
#define RFTYPE_INFO BIT(0)
#define RFTYPE_BASE BIT(1)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 67b315f3a26c..8be0fb323ad3 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -779,6 +779,22 @@ 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;
+}
+
#ifdef CONFIG_PROC_CPU_RESCTRL
/*
@@ -1863,6 +1879,13 @@ static struct rftype res_common_files[] = {
.seq_show = rdtgroup_size_show,
.fflags = RFTYPE_CTRL_BASE,
},
+ {
+ .name = "ctrl_hw_id",
+ .mode = 0444,
+ .kf_ops = &rdtgroup_kf_single_ops,
+ .seq_show = rdtgroup_closid_show,
+ .fflags = RFTYPE_CTRL_BASE | RFTYPE_DEBUG,
+ },
};
--
2.34.1Hi, Babu,
On 9/7/23 16:51, Babu Moger wrote:
> In x86, hardware uses CLOSID to identify a control group. When a user
> creates a control group this information is not visible to the user.
> It can help resctrl debugging.
>
> Add CLOSID(ctrl_hw_id) to the control groups display in resctrl
> interface. Users can see this detail when resctrl is mounted with
> "-o debug" option.
>
> Other architectures do not use "CLOSID". Use the names ctrl_hw_id
> to refer to "CLOSID" in an effort to keep the naming generic.
>
> For example:
> $cat /sys/fs/resctrl/ctrl_grp1/ctrl_hw_id
> 1
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> Documentation/arch/x86/resctrl.rst | 4 ++++
> arch/x86/kernel/cpu/resctrl/internal.h | 3 +++
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 23 +++++++++++++++++++++++
> 3 files changed, 30 insertions(+)
>
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index 28d35aaa74b4..54691c8b832d 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -352,6 +352,10 @@ 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".
>
> +"ctrl_hw_id":
> + Available only with debug option. The identifier used by hardware
> + for the control group. On x86 this is the CLOSID.
> +
> When monitoring is enabled all MON groups will also contain:
>
> "mon_data":
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 38b99a21ccd8..ccdbed615d41 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -299,6 +299,9 @@ struct rdtgroup {
> * --> RFTYPE_CTRL (Files only for CTRL group)
> * Files: mode, schemata, size
> *
> + * --> RFTYPE_DEBUG (Files to help resctrl debugging)
> + * File: ctrl_hw_id
> + *
> */
> #define RFTYPE_INFO BIT(0)
> #define RFTYPE_BASE BIT(1)
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 67b315f3a26c..8be0fb323ad3 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -779,6 +779,22 @@ 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;
> +}
> +
> #ifdef CONFIG_PROC_CPU_RESCTRL
>
> /*
> @@ -1863,6 +1879,13 @@ static struct rftype res_common_files[] = {
> .seq_show = rdtgroup_size_show,
> .fflags = RFTYPE_CTRL_BASE,
> },
> + {
> + .name = "ctrl_hw_id",
> + .mode = 0444,
> + .kf_ops = &rdtgroup_kf_single_ops,
> + .seq_show = rdtgroup_closid_show,
Is it better to rename "rdtgroup_closid_show" as arch neutral name
"rdtgroup_ctrl_hw_id_show"? So the name is arch neutral and reflects the
ctrl_hw_id. So this can eventually go to generic fs code without
renaming it. The getting closid implementation in the function will be
arch specific.
> + .fflags = RFTYPE_CTRL_BASE | RFTYPE_DEBUG,
> + },
>
> };
>
Thanks.
-Fenghua
Hi Fenghua,
On 9/11/2023 11:08 AM, Fenghua Yu wrote:
> On 9/7/23 16:51, Babu Moger wrote:
...
>> @@ -1863,6 +1879,13 @@ static struct rftype res_common_files[] = {
>> .seq_show = rdtgroup_size_show,
>> .fflags = RFTYPE_CTRL_BASE,
>> },
>> + {
>> + .name = "ctrl_hw_id",
>> + .mode = 0444,
>> + .kf_ops = &rdtgroup_kf_single_ops,
>> + .seq_show = rdtgroup_closid_show,
>
> Is it better to rename "rdtgroup_closid_show" as arch neutral name
> "rdtgroup_ctrl_hw_id_show"? So the name is arch neutral and reflects
> the ctrl_hw_id. So this can eventually go to generic fs code without
> renaming it. The getting closid implementation in the function will
> be arch specific.
This is not so obvious to me. We have to draw the line somewhere. This
series draws the line between the kernel and user interface. That is,
from user perspective it is "ctrl_hw_id" and from kernel perspective
it is "closid".
I think renaming rdtgroup_closid_show() to rdtgroup_ctrl_hw_id_show() may
be mixing up the two because the function called rdtgroup_ctrl_hw_id_show()
would operate on rdtgroup->closid - so in a sense renaming the function
to rdtgroup_ctrl_hw_id_show() would move the name further from what the
function does. Would that mean that we need to refactor the whole kernel
the naming to match between kernel space and user space by changing
rdtgroup->closid to rdtgroup->ctrl_hw_id? That escalates quite far and
wide and I wonder if it is not just simpler to keep the naming
boundary between kernel space and user space.
Reinette
© 2016 - 2025 Red Hat, Inc.