Hi Fenghua,
On 9/11/23 13:14, Fenghua Yu wrote:
> Hi, Babu,
>
> On 9/7/23 16:51, Babu Moger wrote:
>> In x86, hardware uses RMID to identify a a monitoring group. When a
>
> s/a a/a/
Sure.
Thanks for the review. Reinette has already addressed other comments.
Thanks
Babu
>
>> user creates a monitor group these details are not visible. These details
>> can help resctrl debugging.
>>
>> Add RMID(mon_hw_id) to the monitor groups display in resctrl interface.
>> Users can see these details when resctrl is mounted with "-o debug" option.
>>
>> Other architectures do not use "RMID". Use the name mon_hw_id to refer
>> to "RMID" in an effort to keep the naming generic.
>>
>> Add the flag RFTYPE_MON_BASE, which contains the files required only
>> for the MON group.
>>
>> For example:
>> $cat /sys/fs/resctrl/mon_groups/mon_grp1/mon_hw_id
>> 3
>>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>> Documentation/arch/x86/resctrl.rst | 4 +++
>> arch/x86/kernel/cpu/resctrl/internal.h | 6 ++++
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 38 +++++++++++++++++++++++---
>> 3 files changed, 44 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst
>> b/Documentation/arch/x86/resctrl.rst
>> index 54691c8b832d..98b0eb509ed4 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -369,6 +369,10 @@ When monitoring is enabled all MON groups will also
>> contain:
>> the sum for all tasks in the CTRL_MON group and all tasks in
>> MON groups. Please see example section for more details on usage.
>> +"mon_hw_id":
>> + Available only with debug option. The identifier used by hardware
>> + for the monitor group. On x86 this is the RMID.
>> +
>> Resource allocation rules
>> -------------------------
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h
>> b/arch/x86/kernel/cpu/resctrl/internal.h
>> index ccdbed615d41..b4910892b0a6 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -296,6 +296,11 @@ struct rdtgroup {
>> * --> RFTYPE_BASE (Files common for both MON and CTRL groups)
>> * Files: cpus, cpus_list, tasks
>> *
>> + * --> RFTYPE_MON (Files only for MON group)
>> + *
>> + * --> RFTYPE_DEBUG (Files to help resctrl debugging)
>> + * File: mon_hw_id
>> + *
>> * --> RFTYPE_CTRL (Files only for CTRL group)
>> * Files: mode, schemata, size
>> *
>> @@ -315,6 +320,7 @@ struct rdtgroup {
>> #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_MON_BASE (RFTYPE_BASE | RFTYPE_MON)
>> /* 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 8be0fb323ad3..fc830ffce82a 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -795,6 +795,22 @@ static int rdtgroup_closid_show(struct
>> kernfs_open_file *of,
>> 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
>> /*
>> @@ -1856,6 +1872,13 @@ static struct rftype res_common_files[] = {
>> .seq_show = rdtgroup_tasks_show,
>> .fflags = RFTYPE_BASE,
>> },
>> + {
>> + .name = "mon_hw_id",
>> + .mode = 0444,
>> + .kf_ops = &rdtgroup_kf_single_ops,
>> + .seq_show = rdtgroup_rmid_show,
>
> Similar to showing ctrl_hw_id, is it better to rename "rdtgroup_rmid_show"
> as "rdtgroup_mon_hw_id_show" for arch neutral naming?
>
>> + .fflags = RFTYPE_MON_BASE | RFTYPE_DEBUG,
>> + },
>> {
>> .name = "schemata",
>> .mode = 0644,
>> @@ -2535,6 +2558,7 @@ static void schemata_list_destroy(void)
>> static int rdt_get_tree(struct fs_context *fc)
>> {
>> struct rdt_fs_context *ctx = rdt_fc2context(fc);
>> + unsigned long flags = RFTYPE_CTRL_BASE;
>> struct rdt_domain *dom;
>> struct rdt_resource *r;
>> int ret;
>> @@ -2565,7 +2589,10 @@ static int rdt_get_tree(struct fs_context *fc)
>> closid_init();
>> - ret = rdtgroup_add_files(rdtgroup_default.kn, RFTYPE_CTRL_BASE);
>> + if (rdt_mon_capable)
>> + flags |= RFTYPE_MON;
>> +
>> + ret = rdtgroup_add_files(rdtgroup_default.kn, flags);
>> if (ret)
>> goto out_schemata_free;
>> @@ -3255,8 +3282,8 @@ static int mkdir_rdt_prepare(struct kernfs_node
>> *parent_kn,
>> enum rdt_group_type rtype, struct rdtgroup **r)
>> {
>> struct rdtgroup *prdtgrp, *rdtgrp;
>> + unsigned long files = 0;
>> struct kernfs_node *kn;
>> - uint files = 0;
>> int ret;
>> prdtgrp = rdtgroup_kn_lock_live(parent_kn);
>> @@ -3308,10 +3335,13 @@ static int mkdir_rdt_prepare(struct kernfs_node
>> *parent_kn,
>> goto out_destroy;
>> }
>> - if (rtype == RDTCTRL_GROUP)
>> + if (rtype == RDTCTRL_GROUP) {
>> files = RFTYPE_BASE | RFTYPE_CTRL;
>> - else
>> + if (rdt_mon_capable)
>> + files |= RFTYPE_MON;
>> + } else {
>> files = RFTYPE_BASE | RFTYPE_MON;
>> + }
>> ret = rdtgroup_add_files(kn, files);
>> if (ret) {
>
> Thanks.
>
> -Fenghua
--
Thanks
Babu Moger
