diff mbox series

[v4,4/7] x86/resctrl: Re-arrange RFTYPE flags and add more comments

Message ID 168177447844.1758847.17789021842152864417.stgit@bmoger-ubuntu
State New
Headers
Series x86/resctrl: Miscellaneous resctrl features |

Commit Message

Moger, Babu April 17, 2023, 11:34 p.m. UTC 
  Remove gaps in bit definitions of RFTYPE flags and add more comments
to make it easy for future additions.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h |   49 +++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 5 deletions(-)

Comments

Reinette Chatre May 4, 2023, 7 p.m. UTC | #1 
Hi Babu,

On 4/17/2023 4:34 PM, Babu Moger wrote:
> Remove gaps in bit definitions of RFTYPE flags and add more comments

Why is it necessary to remove gaps in the bit definitions?

Reinette
Moger, Babu May 5, 2023, 8:40 p.m. UTC | #2 
Hi Reinette,

On 5/4/2023 2:00 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 4/17/2023 4:34 PM, Babu Moger wrote:
>> Remove gaps in bit definitions of RFTYPE flags and add more comments
> Why is it necessary to remove gaps in the bit definitions?

Removing the gaps is not necessary definitely. I thought adding comments 
will help adding new flags in the future.

If you want me to drop this whole patch, I am fine with it.

Thanks

Babu
Reinette Chatre May 5, 2023, 9:24 p.m. UTC | #3 
Hi Babu,

On 5/5/2023 1:40 PM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 5/4/2023 2:00 PM, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 4/17/2023 4:34 PM, Babu Moger wrote:
>>> Remove gaps in bit definitions of RFTYPE flags and add more comments
>> Why is it necessary to remove gaps in the bit definitions?
> 
> Removing the gaps is not necessary definitely. I thought adding
> comments will help adding new flags in the future.
> 

I agree that removing the gaps are not necessary.

> If you want me to drop this whole patch, I am fine with it.> 

The comments may be useful. If you decide to keep it please review
it for consistency. The comments should not increase confusion.
For example,
* in one instance you refer to "info" and "base" as components, in
  another you refer to them as directories, which is confusing since
  there is a "info" directory but no "base" directory.
* related to previous item, the comments start by referring to the
  "info" and "base" components but then the comments switch to
  describing a "info directory structure and "group structure"
* the separator (---) is used above a header in one instance and
  below a header in another
* in some places you use the syntax:
	--> <flag name> (<dir name>, <dir name>)
  in other places you use:
	--> <flag name>
	   --> (<dir name>)
	   --> (<dir name>)

Reinette
Moger, Babu May 9, 2023, 5:33 p.m. UTC | #4 
Hi Reinette,

On 5/5/23 16:24, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/5/2023 1:40 PM, Moger, Babu wrote:
>> Hi Reinette,
>>
>> On 5/4/2023 2:00 PM, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 4/17/2023 4:34 PM, Babu Moger wrote:
>>>> Remove gaps in bit definitions of RFTYPE flags and add more comments
>>> Why is it necessary to remove gaps in the bit definitions?
>>
>> Removing the gaps is not necessary definitely. I thought adding
>> comments will help adding new flags in the future.
>>
> 
> I agree that removing the gaps are not necessary.
ok.

> 
>> If you want me to drop this whole patch, I am fine with it.> 
> 
> The comments may be useful. If you decide to keep it please review
> it for consistency. The comments should not increase confusion.
> For example,
> * in one instance you refer to "info" and "base" as components, in
>   another you refer to them as directories, which is confusing since
>   there is a "info" directory but no "base" directory.
> * related to previous item, the comments start by referring to the
>   "info" and "base" components but then the comments switch to
>   describing a "info directory structure and "group structure"
> * the separator (---) is used above a header in one instance and
>   below a header in another
> * in some places you use the syntax:
> 	--> <flag name> (<dir name>, <dir name>)
>   in other places you use:
> 	--> <flag name>
> 	   --> (<dir name>)
> 	   --> (<dir name>)
> 
>
sure. Will address this next revision.

Thanks
Babu
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index c4fc5a1c630c..75ddbd833fdf 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -240,14 +240,53 @@  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
+ *     --> RFTYPE_INFO
+ *         --> RFTYPE_TOP_INFO
+ *             last_cmd_status
+ *
+ *         --> RFTYPE_MON_INFO
+ *            --> (L2_MON)
+ *            --> (L3_MON)
+ *                max_threshold_occupancy, mbm_local_bytes_config,
+ *                mbm_total_bytes_config, mon_features, num_rmids
+ *
+ *         --> RFTYPE_CTRL_INFO
+ *            --> RFTYPE_RES_CACHE (L2, L3)
+ *                bit_usage, cbm_mask, min_cbm_bits, num_closids,
+ *                shareable_bits
+ *
+ *            --> RFTYPE_RES_MB (MB, SMBA)
+ *                bandwidth_gran, delay_linear, min_bandwidth,
+ *                num_closids
+ *
+ *     group structure
+ *     -----------------------------------------------------------
+ *     --> RFTYPE_BASE (Files common for both MON and CTRL groups)
+ *         cpus, cpus_list, tasks
+ *
+ *     --> RFTYPE_CTRL_BASE (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			BIT(2)
+#define RFTYPE_MON			BIT(3)
+#define RFTYPE_TOP			BIT(4)
+#define RFTYPE_RES_CACHE		BIT(5)
+#define RFTYPE_RES_MB			BIT(6)
 #define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
 #define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
 #define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)