[v4,3/7] x86/resctrl: Rename rftype flags for consistency

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

Commit Message

Moger, Babu April 17, 2023, 11:34 p.m. UTC
  The rftype flags have two different prefixes even though they are used
for the same purpose. Change the prefix to RFTYPE_ for all the flags.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/internal.h |    8 +++---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   42 ++++++++++++++++----------------
 2 files changed, 25 insertions(+), 25 deletions(-)
  

Comments

Ilpo Järvinen April 19, 2023, 12:44 p.m. UTC | #1
On Mon, 17 Apr 2023, Babu Moger wrote:

> The rftype flags have two different prefixes even though they are used
> for the same purpose. Change the prefix to RFTYPE_ for all the flags.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/internal.h |    8 +++---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   42 ++++++++++++++++----------------
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index 62767774810d..c4fc5a1c630c 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -248,10 +248,10 @@ 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)
>  
>  /* List of all resource groups */
>  extern struct list_head rdt_all_groups;

This needs to be changed as well:

 * @fflags:  File specific RF_* or RFTYPE_* flags
  
Moger, Babu April 19, 2023, 2:29 p.m. UTC | #2
Hi Jarvinen,

On 4/19/23 07:44, Ilpo Järvinen wrote:
> On Mon, 17 Apr 2023, Babu Moger wrote:
> 
>> The rftype flags have two different prefixes even though they are used
>> for the same purpose. Change the prefix to RFTYPE_ for all the flags.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/resctrl/internal.h |    8 +++---
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   42 ++++++++++++++++----------------
>>  2 files changed, 25 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 62767774810d..c4fc5a1c630c 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -248,10 +248,10 @@ 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)
>>  
>>  /* List of all resource groups */
>>  extern struct list_head rdt_all_groups;
> 
> This needs to be changed as well:
> 
>  * @fflags:  File specific RF_* or RFTYPE_* flags

Yes. Thanks for pointing that. Will correct it in next revision.
  
Reinette Chatre May 4, 2023, 7 p.m. UTC | #3
Hi Babu,

On 4/17/2023 4:34 PM, Babu Moger wrote:
> The rftype flags have two different prefixes even though they are used

Could this changelog please start with some context about what the
rftype flags are? Please note that the changelogs are required
to comply to the x86 requirements as documented in:
Documentation/process/maintainer-tip.rst (there is a whole
section on changelogs).

After a context the problem could be made specific with something
like "rftype flags use the RF_ as well as RFTYPE_ prefixes."

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

On 5/5/2023 1:16 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:
>>> The rftype flags have two different prefixes even though they are used
>> Could this changelog please start with some context about what the
>> rftype flags are? Please note that the changelogs are required
>> to comply to the x86 requirements as documented in:
>> Documentation/process/maintainer-tip.rst (there is a whole
>> section on changelogs).
> Thanks
>>
>> After a context the problem could be made specific with something
>> like "rftype flags use the RF_ as well as RFTYPE_ prefixes."

> I can say start with "rftype flags are used for creating the files
> under resctrl filesystem. These flags  use the RF_ as well as RFTYPE_
> prefixes.
> 
> Change the prefix to RFTYPE_ for all the flags to be consistent."> 

I would be careful with the term "rftype flags" since it is
not clear what is meant by this. Note that there are RF_*, RFTYPE_*,
as well as RFTYPE_FLAGS* flags and the reader may easily think that
you refer to the latter.

"resctrl associates 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 ..."

(A shortcut I used was to look up the commit that introduced the change
and borrow some text from it.)

Reinette
  
Moger, Babu May 9, 2023, 5:42 p.m. UTC | #5
On 5/5/23 16:24, Reinette Chatre wrote:
> Hi Babu,
> 
> On 5/5/2023 1:16 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:
>>>> The rftype flags have two different prefixes even though they are used
>>> Could this changelog please start with some context about what the
>>> rftype flags are? Please note that the changelogs are required
>>> to comply to the x86 requirements as documented in:
>>> Documentation/process/maintainer-tip.rst (there is a whole
>>> section on changelogs).
>> Thanks
>>>
>>> After a context the problem could be made specific with something
>>> like "rftype flags use the RF_ as well as RFTYPE_ prefixes."
> 
>> I can say start with "rftype flags are used for creating the files
>> under resctrl filesystem. These flags  use the RF_ as well as RFTYPE_
>> prefixes.
>>
>> Change the prefix to RFTYPE_ for all the flags to be consistent."> 
> 
> I would be careful with the term "rftype flags" since it is
> not clear what is meant by this. Note that there are RF_*, RFTYPE_*,
> as well as RFTYPE_FLAGS* flags and the reader may easily think that
> you refer to the latter.
> 
> "resctrl associates 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 ..."

Will change it.

> 
> (A shortcut I used was to look up the commit that introduced the change
> and borrow some text from it.)
Sure.

Thanks
Babu Moger
  

Patch

diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 62767774810d..c4fc5a1c630c 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -248,10 +248,10 @@  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)
 
 /* List of all resource groups */
 extern struct list_head rdt_all_groups;
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 6cd0a8396f30..719e29248892 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1711,77 +1711,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
@@ -1800,7 +1800,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",
@@ -1847,7 +1847,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",
@@ -1855,14 +1855,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,
 	},
 
 };
@@ -1919,7 +1919,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)
@@ -1928,7 +1928,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;
 }
 
 /**
@@ -2063,21 +2063,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)
@@ -3578,7 +3578,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;