diff mbox series

[v11,03/10] x86/resctrl: Rename rftype flags for consistency

Message ID 20231003235430.1231238-4-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 associates rftype flags with its files so that files can be chosen
based on the resource, whether it is info or base, and if it is control
or monitor type file. These flags use the RF_ as well as RFTYPE_ prefixes.

Change the prefix to RFTYPE_ for all these flags to be consistent.

Add RFTYPE_MON_BASE that will be used in a later patch. RFTYPE_MON_BASE
complements existing RFTYPE_CTRL_BASE and represents files
belonging to monitoring groups.

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 | 11 ++++---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 42 +++++++++++++-------------
 2 files changed, 27 insertions(+), 26 deletions(-)

Comments

Borislav Petkov Oct. 9, 2023, 5:19 p.m. UTC | #1 
On Tue, Oct 03, 2023 at 06:54:23PM -0500, Babu Moger wrote:
> resctrl associates rftype flags with its files so that files can be chosen
> based on the resource, whether it is info or base, and if it is control
> or monitor type file. These flags use the RF_ as well as RFTYPE_ prefixes.
> 
> Change the prefix to RFTYPE_ for all these flags to be consistent.
> 
> Add RFTYPE_MON_BASE that will be used in a later patch. RFTYPE_MON_BASE
> complements existing RFTYPE_CTRL_BASE and represents files
> belonging to monitoring groups.

First of all, the concept of "later patch" has no meaning when the
patches go into git, depending on the listing order.

Second, why isn't this define added in the patch that's using it?

A patch needs to be a logically separate, possibly self-contained
change - remember that.

Thx.
Reinette Chatre Oct. 9, 2023, 5:48 p.m. UTC | #2 
Hi Boris,

On 10/9/2023 10:19 AM, Borislav Petkov wrote:
> On Tue, Oct 03, 2023 at 06:54:23PM -0500, Babu Moger wrote:
>> resctrl associates rftype flags with its files so that files can be chosen
>> based on the resource, whether it is info or base, and if it is control
>> or monitor type file. These flags use the RF_ as well as RFTYPE_ prefixes.
>>
>> Change the prefix to RFTYPE_ for all these flags to be consistent.
>>
>> Add RFTYPE_MON_BASE that will be used in a later patch. RFTYPE_MON_BASE
>> complements existing RFTYPE_CTRL_BASE and represents files
>> belonging to monitoring groups.
> 
> First of all, the concept of "later patch" has no meaning when the
> patches go into git, depending on the listing order.
> 
> Second, why isn't this define added in the patch that's using it?
> 
> A patch needs to be a logically separate, possibly self-contained
> change - remember that.

My apologies here. Babu was following my guidance on this. 

Reinette
Borislav Petkov Oct. 9, 2023, 5:53 p.m. UTC | #3 
On Mon, Oct 09, 2023 at 10:48:38AM -0700, Reinette Chatre wrote:
> My apologies here. Babu was following my guidance on this.

No worries, that's the "next level review" kicking in. :-)
Moger, Babu Oct. 9, 2023, 7:22 p.m. UTC | #4 
Hi Boris,

On 10/9/23 12:19, Borislav Petkov wrote:
> On Tue, Oct 03, 2023 at 06:54:23PM -0500, Babu Moger wrote:
>> resctrl associates rftype flags with its files so that files can be chosen
>> based on the resource, whether it is info or base, and if it is control
>> or monitor type file. These flags use the RF_ as well as RFTYPE_ prefixes.
>>
>> Change the prefix to RFTYPE_ for all these flags to be consistent.
>>
>> Add RFTYPE_MON_BASE that will be used in a later patch. RFTYPE_MON_BASE
>> complements existing RFTYPE_CTRL_BASE and represents files
>> belonging to monitoring groups.
> 
> First of all, the concept of "later patch" has no meaning when the
> patches go into git, depending on the listing order.
> 
> Second, why isn't this define added in the patch that's using it?

Ok. Will move RFTYPE_MON_BASE patch 10. Will update the text accordingly.

> 
> A patch needs to be a logically separate, possibly self-contained
> change - remember that.
> 
> Thx.
>
Moger, Babu Oct. 9, 2023, 7:24 p.m. UTC | #5 
Hi Boris,

On 10/9/23 12:19, Borislav Petkov wrote:
> On Tue, Oct 03, 2023 at 06:54:23PM -0500, Babu Moger wrote:
>> resctrl associates rftype flags with its files so that files can be chosen
>> based on the resource, whether it is info or base, and if it is control
>> or monitor type file. These flags use the RF_ as well as RFTYPE_ prefixes.
>>
>> Change the prefix to RFTYPE_ for all these flags to be consistent.
>>
>> Add RFTYPE_MON_BASE that will be used in a later patch. RFTYPE_MON_BASE
>> complements existing RFTYPE_CTRL_BASE and represents files
>> belonging to monitoring groups.
> 
> First of all, the concept of "later patch" has no meaning when the
> patches go into git, depending on the listing order.
> 
> Second, why isn't this define added in the patch that's using it?

ok. Will move RFTYPE_MON_BASE to patch 10 where it is used and will
changes the text accordingly.

> 
> A patch needs to be a logically separate, possibly self-contained
> change - remember that.
> 
> Thx.
>
diff mbox series

Patch

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 62767774810d..f71bc82c882f 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -248,10 +248,11 @@  struct rdtgroup {
 #define RFTYPE_TOP			BIT(6)
 #define RFTYPE_RES_CACHE		BIT(8)
 #define RFTYPE_RES_MB			BIT(9)
-#define RF_CTRL_INFO			(RFTYPE_INFO | RFTYPE_CTRL)
-#define RF_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
-#define RF_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
-#define RF_CTRL_BASE			(RFTYPE_BASE | RFTYPE_CTRL)
+#define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
+#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;
@@ -267,7 +268,7 @@  void __exit rdtgroup_exit(void);
  * @mode:	Access mode
  * @kf_ops:	File operations
  * @flags:	File specific RFTYPE_FLAGS_* flags
- * @fflags:	File specific RF_* or RFTYPE_* flags
+ * @fflags:	File specific RFTYPE_* flags
  * @seq_show:	Show content of the file
  * @write:	Write to the file
  */
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 7ddfa4b470e6..35945b4bf196 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1705,77 +1705,77 @@  static struct rftype res_common_files[] = {
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_last_cmd_status_show,
-		.fflags		= RF_TOP_INFO,
+		.fflags		= RFTYPE_TOP_INFO,
 	},
 	{
 		.name		= "num_closids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_closids_show,
-		.fflags		= RF_CTRL_INFO,
+		.fflags		= RFTYPE_CTRL_INFO,
 	},
 	{
 		.name		= "mon_features",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_mon_features_show,
-		.fflags		= RF_MON_INFO,
+		.fflags		= RFTYPE_MON_INFO,
 	},
 	{
 		.name		= "num_rmids",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_num_rmids_show,
-		.fflags		= RF_MON_INFO,
+		.fflags		= RFTYPE_MON_INFO,
 	},
 	{
 		.name		= "cbm_mask",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_default_ctrl_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "min_cbm_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_cbm_bits_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "shareable_bits",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_shareable_bits_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "bit_usage",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bit_usage_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "min_bandwidth",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_min_bw_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	{
 		.name		= "bandwidth_gran",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_bw_gran_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	{
 		.name		= "delay_linear",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdt_delay_linear_show,
-		.fflags		= RF_CTRL_INFO | RFTYPE_RES_MB,
+		.fflags		= RFTYPE_CTRL_INFO | RFTYPE_RES_MB,
 	},
 	/*
 	 * Platform specific which (if any) capabilities are provided by
@@ -1794,7 +1794,7 @@  static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= max_threshold_occ_write,
 		.seq_show	= max_threshold_occ_show,
-		.fflags		= RF_MON_INFO | RFTYPE_RES_CACHE,
+		.fflags		= RFTYPE_MON_INFO | RFTYPE_RES_CACHE,
 	},
 	{
 		.name		= "mbm_total_bytes_config",
@@ -1841,7 +1841,7 @@  static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_schemata_write,
 		.seq_show	= rdtgroup_schemata_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 	{
 		.name		= "mode",
@@ -1849,14 +1849,14 @@  static struct rftype res_common_files[] = {
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.write		= rdtgroup_mode_write,
 		.seq_show	= rdtgroup_mode_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 	{
 		.name		= "size",
 		.mode		= 0444,
 		.kf_ops		= &rdtgroup_kf_single_ops,
 		.seq_show	= rdtgroup_size_show,
-		.fflags		= RF_CTRL_BASE,
+		.fflags		= RFTYPE_CTRL_BASE,
 	},
 
 };
@@ -1913,7 +1913,7 @@  void __init thread_throttle_mode_init(void)
 	if (!rft)
 		return;
 
-	rft->fflags = RF_CTRL_INFO | RFTYPE_RES_MB;
+	rft->fflags = RFTYPE_CTRL_INFO | RFTYPE_RES_MB;
 }
 
 void __init mbm_config_rftype_init(const char *config)
@@ -1922,7 +1922,7 @@  void __init mbm_config_rftype_init(const char *config)
 
 	rft = rdtgroup_get_rftype_by_name(config);
 	if (rft)
-		rft->fflags = RF_MON_INFO | RFTYPE_RES_CACHE;
+		rft->fflags = RFTYPE_MON_INFO | RFTYPE_RES_CACHE;
 }
 
 /**
@@ -2057,21 +2057,21 @@  static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn)
 	if (IS_ERR(kn_info))
 		return PTR_ERR(kn_info);
 
-	ret = rdtgroup_add_files(kn_info, RF_TOP_INFO);
+	ret = rdtgroup_add_files(kn_info, RFTYPE_TOP_INFO);
 	if (ret)
 		goto out_destroy;
 
 	/* loop over enabled controls, these are all alloc_capable */
 	list_for_each_entry(s, &resctrl_schema_all, list) {
 		r = s->res;
-		fflags =  r->fflags | RF_CTRL_INFO;
+		fflags = r->fflags | RFTYPE_CTRL_INFO;
 		ret = rdtgroup_mkdir_info_resdir(s, s->name, fflags);
 		if (ret)
 			goto out_destroy;
 	}
 
 	for_each_mon_capable_rdt_resource(r) {
-		fflags =  r->fflags | RF_MON_INFO;
+		fflags = r->fflags | RFTYPE_MON_INFO;
 		sprintf(name, "%s_MON", r->name);
 		ret = rdtgroup_mkdir_info_resdir(r, name, fflags);
 		if (ret)
@@ -3709,7 +3709,7 @@  static int __init rdtgroup_setup_root(void)
 
 	list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
 
-	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RF_CTRL_BASE);
+	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_CTRL_BASE);
 	if (ret) {
 		kernfs_destroy_root(rdt_root);
 		goto out;