[v11,04/10] x86/resctrl: Add comments on RFTYPE flags hierarchy

Message ID 20231003235430.1231238-5-babu.moger@amd.com
State New
Headers
Series x86/resctrl: Miscellaneous resctrl features |

Commit Message

Moger, Babu Oct. 3, 2023, 11:54 p.m. UTC
  resctrl uses RFTYPE flags for creating resctrl directory structure.

Definitions and directory structures are not documented. Add
comments to improve the readability and help future additions.

Signed-off-by: Babu Moger <babu.moger@amd.com>
Tested-by: Peter Newman <peternewman@google.com>
Reviewed-by: Peter Newman <peternewman@google.com>
Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h | 58 ++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
  

Comments

Borislav Petkov Oct. 9, 2023, 5:22 p.m. UTC | #1
On Tue, Oct 03, 2023 at 06:54:24PM -0500, Babu Moger wrote:
> resctrl uses RFTYPE flags for creating resctrl directory structure.
> 
> Definitions and directory structures are not documented. Add
> comments to improve the readability and help future additions.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Tested-by: Peter Newman <peternewman@google.com>
> Reviewed-by: Peter Newman <peternewman@google.com>
> Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/internal.h | 58 ++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index f71bc82c882f..b47a5906f952 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -240,6 +240,64 @@ struct rdtgroup {
>  
>  /*
>   * Define the file type flags for base and info directories.

Why is this lengthy comment explaining internals in a header and not in
the perfectly fine

Documentation/arch/x86/resctrl.rst

file?
  
Moger, Babu Oct. 9, 2023, 7:39 p.m. UTC | #2
Hi Boris,

On 10/9/23 12:22, Borislav Petkov wrote:
> On Tue, Oct 03, 2023 at 06:54:24PM -0500, Babu Moger wrote:
>> resctrl uses RFTYPE flags for creating resctrl directory structure.
>>
>> Definitions and directory structures are not documented. Add
>> comments to improve the readability and help future additions.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> Tested-by: Peter Newman <peternewman@google.com>
>> Reviewed-by: Peter Newman <peternewman@google.com>
>> Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
>> Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
>> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
>> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> ---
>>  arch/x86/kernel/cpu/resctrl/internal.h | 58 ++++++++++++++++++++++++++
>>  1 file changed, 58 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index f71bc82c882f..b47a5906f952 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -240,6 +240,64 @@ struct rdtgroup {
>>  
>>  /*
>>   * Define the file type flags for base and info directories.
> 
> Why is this lengthy comment explaining internals in a header and not in
> the perfectly fine
> 
> Documentation/arch/x86/resctrl.rst
> 
> file?
> 

User visible files are already documented in resctrl.rst file.

Understanding of these flags are mostly required for programmers. Users
don't need to know all these internal flags. That is why it is listed in here.
  
Borislav Petkov Oct. 9, 2023, 7:51 p.m. UTC | #3
On Mon, Oct 09, 2023 at 02:39:30PM -0500, Moger, Babu wrote:
> User visible files are already documented in resctrl.rst file.
> 
> Understanding of these flags are mostly required for programmers. Users
> don't need to know all these internal flags. That is why it is listed in here.

You can have a section in resctrl.rst called "Implementation notes" or
so. This way you have it all in one file instead of having programmers
grep the whole tree for info.
  
Moger, Babu Oct. 9, 2023, 8:36 p.m. UTC | #4
Hi Boris,

On 10/9/23 14:51, Borislav Petkov wrote:
> On Mon, Oct 09, 2023 at 02:39:30PM -0500, Moger, Babu wrote:
>> User visible files are already documented in resctrl.rst file.
>>
>> Understanding of these flags are mostly required for programmers. Users
>> don't need to know all these internal flags. That is why it is listed in here.
> 
> You can have a section in resctrl.rst called "Implementation notes" or
> so. This way you have it all in one file instead of having programmers
> grep the whole tree for info.
> 

Will add new section in resctrl.rst to add these details. Will send v12 soon.
  

Patch

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index f71bc82c882f..b47a5906f952 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -240,6 +240,64 @@  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 (Top level directory named "info". Contains files that
+ *	|	  provide details on control and monitoring resources.)
+ *	|
+ *	--> base (Root directory associated with default resource group
+ *		  as well as directories created by user for MON and CTRL
+ *		  groups. Contains files to interact with MON and CTRL
+ *		  groups.)
+ *
+ *	Note: resctrl uses flags for files, not for directories.
+ *	      Directories are created based on the resource type. Added
+ *	      directories below for better understanding.
+ *
+ *	info directory structure
+ *	------------------------------------------------------------------
+ *	--> RFTYPE_INFO
+ *	    Directory: info
+ *		--> RFTYPE_TOP (Files in top level of info directory)
+ *		    File: last_cmd_status
+ *
+ *		--> RFTYPE_MON (Files for all monitoring resources)
+ *		    Directory: L3_MON
+ *		        Files: mon_features, num_rmids
+ *
+ *			--> RFTYPE_RES_CACHE (Files for cache monitoring resources)
+ *			    Directory: L3_MON
+ *			        Files: max_threshold_occupancy,
+ *			               mbm_total_bytes_config,
+ *			               mbm_local_bytes_config
+ *
+ *		--> RFTYPE_CTRL (Files for all control resources)
+ *		    Directories: L2, L3, MB, SMBA, L2CODE, L2DATA, L3CODE, L3DATA
+ *		           File: num_closids
+ *
+ *			--> RFTYPE_RES_CACHE (Files for cache control resources)
+ *			    Directories: L2, L3, L2CODE, L2DATA, L3CODE, L3DATA
+ *			          Files: bit_usage, cbm_mask, min_cbm_bits,
+ *			                 shareable_bits
+ *
+ *			--> RFTYPE_RES_MB (Files for memory control resources)
+ *			    Directories: MB, SMBA
+ *			          Files: bandwidth_gran, delay_linear,
+ *			                 min_bandwidth, thread_throttle_mode
+ *
+ *	base directory structure
+ *	------------------------------------------------------------------
+ *	--> RFTYPE_BASE (Files common for both MON and CTRL groups)
+ *	    Files: cpus, cpus_list, tasks
+ *
+ *		--> RFTYPE_CTRL (Files for CTRL group)
+ *		    Files: mode, schemata, size
+ *
  */
 #define RFTYPE_INFO			BIT(0)
 #define RFTYPE_BASE			BIT(1)