Add the debug files to the resctrl hierarchy.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   48 +++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index a1f13715a65c..790c6b9f9031 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -2400,6 +2400,45 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
 			     struct rdtgroup *prgrp,
 			     struct kernfs_node **mon_data_kn);
 
+void resctrl_add_debug_file(struct kernfs_node *parent_kn, const char *config,
+			    unsigned long fflags)
+{
+	struct rftype *rft;
+
+	rft = rdtgroup_get_rftype_by_name(config);
+	if (rft) {
+		rft->fflags |= fflags;
+		rdtgroup_add_file(parent_kn, rft);
+	}
+}
+
+static void resctrl_add_debug_files(void)
+{
+	resctrl_add_debug_file(rdtgroup_default.kn, "rmid", RFTYPE_BASE);
+	resctrl_add_debug_file(rdtgroup_default.kn, "closid", RFTYPE_BASE_CTRL);
+	kernfs_activate(rdtgroup_default.kn);
+}
+
+void resctrl_remove_debug_file(struct kernfs_node *parent_kn,
+			       const char *config, unsigned long fflags)
+{
+	struct rftype *rft;
+
+	rft = rdtgroup_get_rftype_by_name(config);
+	if (rft) {
+		rft->fflags &= ~fflags;
+		kernfs_remove_by_name(parent_kn, config);
+	}
+}
+
+static void resctrl_remove_debug_files(void)
+{
+	resctrl_remove_debug_file(rdtgroup_default.kn, "rmid", RFTYPE_BASE);
+	resctrl_remove_debug_file(rdtgroup_default.kn, "closid",
+				  RFTYPE_BASE_CTRL);
+	kernfs_activate(rdtgroup_default.kn);
+}
+
 static int rdt_enable_ctx(struct rdt_fs_context *ctx)
 {
 	int ret = 0;
@@ -2413,8 +2452,10 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx)
 	if (!ret && ctx->enable_mba_mbps)
 		ret = set_mba_sc(true);
 
-	if (!ret && ctx->debug)
+	if (!ret && ctx->debug) {
 		resctrl_debug = true;
+		resctrl_add_debug_files();
+	}
 
 	return ret;
 }
@@ -2831,6 +2872,11 @@ static void rdt_kill_sb(struct super_block *sb)
 
 	set_mba_sc(false);
 
+	if (resctrl_debug) {
+		resctrl_remove_debug_files();
+		resctrl_debug = false;
+	}
+
 	/*Put everything back to default values. */
 	for_each_alloc_capable_rdt_resource(r)
 		reset_all_ctrls(r);

Hi Babu,

On 3/2/2023 12:25 PM, Babu Moger wrote:
> Add the debug files to the resctrl hierarchy.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   48 +++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index a1f13715a65c..790c6b9f9031 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2400,6 +2400,45 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>  			     struct rdtgroup *prgrp,
>  			     struct kernfs_node **mon_data_kn);
>  
> +void resctrl_add_debug_file(struct kernfs_node *parent_kn, const char *config,
> +			    unsigned long fflags)
> +{
> +	struct rftype *rft;
> +
> +	rft = rdtgroup_get_rftype_by_name(config);
> +	if (rft) {
> +		rft->fflags |= fflags;
> +		rdtgroup_add_file(parent_kn, rft);
> +	}
> +}
> +
> +static void resctrl_add_debug_files(void)
> +{
> +	resctrl_add_debug_file(rdtgroup_default.kn, "rmid", RFTYPE_BASE);
> +	resctrl_add_debug_file(rdtgroup_default.kn, "closid", RFTYPE_BASE_CTRL);
> +	kernfs_activate(rdtgroup_default.kn);
> +}
> +

I think that separating this from all the other resctrl file creation
can be a source of confusion and bugs. Why not add the debug files
at the same time as all the other files belonging to a resource group?

How about introducing new flags, perhaps RFTYPE_MON_DEBUG and
RFTYPE_CTRL_DEBUG. When the debug option is enabled (when resctrl_debug
is true) then the appropriate flag can be OR'd with the other flags
before rdtgroup_add_files() is called.

It sounds to me if there are plans to add more of these files. A function
like resctrl_add_debug_files() requires a lot of changes and care (and thus
potential for errors) every time a new debug file is added.

On another note ... what are the plans with this debug area? At some
point it may be better to expand resctrl debugfs.

Reinette

On 3/15/23 13:45, Reinette Chatre wrote:
> Hi Babu,
> 
> On 3/2/2023 12:25 PM, Babu Moger wrote:
>> Add the debug files to the resctrl hierarchy.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   48 +++++++++++++++++++++++++++++++-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index a1f13715a65c..790c6b9f9031 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2400,6 +2400,45 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>>  			     struct rdtgroup *prgrp,
>>  			     struct kernfs_node **mon_data_kn);
>>  
>> +void resctrl_add_debug_file(struct kernfs_node *parent_kn, const char *config,
>> +			    unsigned long fflags)
>> +{
>> +	struct rftype *rft;
>> +
>> +	rft = rdtgroup_get_rftype_by_name(config);
>> +	if (rft) {
>> +		rft->fflags |= fflags;
>> +		rdtgroup_add_file(parent_kn, rft);
>> +	}
>> +}
>> +
>> +static void resctrl_add_debug_files(void)
>> +{
>> +	resctrl_add_debug_file(rdtgroup_default.kn, "rmid", RFTYPE_BASE);
>> +	resctrl_add_debug_file(rdtgroup_default.kn, "closid", RFTYPE_BASE_CTRL);
>> +	kernfs_activate(rdtgroup_default.kn);
>> +}
>> +
> 
> I think that separating this from all the other resctrl file creation
> can be a source of confusion and bugs. Why not add the debug files
> at the same time as all the other files belonging to a resource group?
> 
> How about introducing new flags, perhaps RFTYPE_MON_DEBUG and
> RFTYPE_CTRL_DEBUG. When the debug option is enabled (when resctrl_debug
> is true) then the appropriate flag can be OR'd with the other flags
> before rdtgroup_add_files() is called.

Yes. I could try that.
> 
> It sounds to me if there are plans to add more of these files. A function
> like resctrl_add_debug_files() requires a lot of changes and care (and thus
> potential for errors) every time a new debug file is added.
> 
> On another note ... what are the plans with this debug area? At some
> point it may be better to expand resctrl debugfs.

Hmm.. I have not thought about that.

-- 
Thanks
Babu Moger