Hi Reinette, On 9/12/23 12:57, Reinette Chatre wrote: > Hi Babu, > > On 9/12/2023 10:39 AM, Moger, Babu wrote: >> On 9/11/23 18:08, Reinette Chatre wrote: >>> Hi Babu, >>> >>> On 9/7/2023 4:51 PM, Babu Moger wrote: >>>> In x86, hardware uses RMID to identify a a monitoring group. When a >>>> 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. >>> >>> As I mentioned in [1] I believe adding support for files with RFTYPE_MON flag >>> to resctrl needs to be in a separate patch. With the core support added a file >>> with this flag can be introduced separately. >> >> Yes. We discussed it. Thought it may not fit as a separate patch on its >> own. So, i combined it. >> >> Now, I split patch 9 into two. Let me know if it looks ok. >> >> ========================================================================== >> >> Author: Babu Moger <babu.moger@xxxxxxx> >> Date: Thu Sep 7 18:51:28 2023 -0500 >> >> x86/resctrl: Add RFTYPE_MON_BASE for MON groups >> >> Add the flag RFTYPE_MON_BASE, which contains the files required only >> for the MON group. Files with these flags are only visible when >> monitoring is enabled. >> >> Signed-off-by: Babu Moger <babu.moger@xxxxxxx> >> >> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h >> b/arch/x86/kernel/cpu/resctrl/internal.h >> index ccdbed615d41..72988e5c52a7 100644 >> --- a/arch/x86/kernel/cpu/resctrl/internal.h >> +++ b/arch/x86/kernel/cpu/resctrl/internal.h >> @@ -296,6 +296,8 @@ 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_CTRL (Files only for CTRL group) >> * Files: mode, schemata, size >> * > > I think it would be ok to add above hunk as part of the patch > that adds mon_hw_id, similar to what you did in patch 8. Ok. Sure. > >> @@ -315,6 +317,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) >> > > This hunk may be appropriate for patch 3. Sure. > >> /* List of all resource groups */ >> extern struct list_head rdt_all_groups; >> > > This is not how I expected it to look. There is currently an issue > in resctrl where files with RFTYPE_MON are not created for CTRL_MON > groups. This has not actually been a problem because there are > no files with the RFTYPE_MON flag. This series introduces the first > file with RFTYPE_MON flag and thus this resctrl issue needs to be fixed > before this new file can be supported. Yes. > > Considering this I expected to see the changes in rdt_get_tree() > and mkdir_rdt_prepare() done as a separate patch. This will add > support for RFTYPE_MON files to resctrl. After this change > a separate patch can add the file that has this flag. > Sure. Got it. Thanks Babu Moger