Hi Reinette,
On 8/29/23 15:14, Reinette Chatre wrote:
> Hi Babu,
>
> On 8/21/2023 4:30 PM, Babu Moger wrote:
>
> ...
>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 2fae6d9e24d3..3fa32c1c2677 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -296,9 +296,15 @@ struct rdtgroup {
>> * --> RFTYPE_BASE (Files common for both MON and CTRL groups)
>> * Files: cpus, cpus_list, tasks
>> *
>> + * --> RFTYPE_DEBUG (Files to help resctrl debugging)
>> + * File: mon_hw_id
>> + *
>
> This does not look right. I think mon_hw_id should have RFTYPE_MON
> (more below).
I am not sure about this (more below).
>
>> * --> 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 94bd69f9964c..e0c2479acf49 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -776,6 +776,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
>>
>> /*
>> @@ -1837,6 +1869,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,
>> + .fflags = RFTYPE_BASE | RFTYPE_DEBUG,
>
> I missed this earlier but I think this also needs a RFTYPE_MON.
> Perhaps patch 3 can introduce a new RFTYPE_MON_BASE to not
> have the flags of the two new files look too different?
We have two types of files in base directory structure.
if (rtype == RDTCTRL_GROUP)
files = RFTYPE_BASE | RFTYPE_CTRL;
else
files = RFTYPE_BASE | RFTYPE_MON;
1. RFTYPE_BASE | RFTYPE_CTRL;
Files for the control group only.
2. RFTYPE_BASE | RFTYPE_MON;
Files for both control and mon groups. However, RFTYPE_MON is not used
for any files. It is only RFTYPE_BASE.
Because of the check in rdtgroup_add_files it all works fine.
For the control group it creates files with RFTYPE_BASE | RFTYPE_CTRL and
RFTYPE_BASE.
For the mon group it creates files with RFTYPE_BASE only.
Adding FTYPE_MON_BASE will be a problem.
--
Thanks
Babu Moger
