[v6,7/8] x86/resctrl: Introduce "-o debug" mount option

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

Commit Message

Moger, Babu July 19, 2023, 11:22 p.m. UTC
  Add "-o debug" option to mount resctrl filesystem in debug mode. This
option is used for adding extra files to help resctrl debugging.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 Documentation/arch/x86/resctrl.rst     |    5 ++++-
 arch/x86/kernel/cpu/resctrl/internal.h |    2 ++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c |   20 ++++++++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)
  

Comments

Reinette Chatre Aug. 4, 2023, 8:42 p.m. UTC | #1
Hi Babu,

On 7/19/2023 4:22 PM, Babu Moger wrote:
> @@ -2591,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
>  out_schemata_free:
>  	schemata_list_destroy();
>  out_ctx:
> +	if (ctx->enable_debug)
> +		resctrl_debug = false;
>  	if (ctx->enable_mba_mbps)
>  		set_mba_sc(false);
>  	cdp_disable_all();

These changes are a red flag to me. Developers still need to
do what patch #4 was aiming to prevent.

Reinette
  
Moger, Babu Aug. 8, 2023, 4:29 p.m. UTC | #2
Hi Reinette,

On 8/4/23 15:42, Reinette Chatre wrote:
> Hi Babu,
> 
> On 7/19/2023 4:22 PM, Babu Moger wrote:
>> @@ -2591,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
>>  out_schemata_free:
>>  	schemata_list_destroy();
>>  out_ctx:
>> +	if (ctx->enable_debug)
>> +		resctrl_debug = false;
>>  	if (ctx->enable_mba_mbps)
>>  		set_mba_sc(false);
>>  	cdp_disable_all();
> 
> These changes are a red flag to me. Developers still need to
> do what patch #4 was aiming to prevent.

I think you meant patch 5 (x86/resctrl: Unwind the errors inside
rdt_enable_ctx).

I can take care of this unwind part in rdt_disable_ctx() and call it here.
https://lore.kernel.org/lkml/9fd70ef3-ca90-65e3-4746-7d574bdd159b@intel.com/#t

Hope that is what you meant.
  
Reinette Chatre Aug. 8, 2023, 5:09 p.m. UTC | #3
Hi Babu,

On 8/8/2023 9:29 AM, Moger, Babu wrote:
> On 8/4/23 15:42, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 7/19/2023 4:22 PM, Babu Moger wrote:
>>> @@ -2591,6 +2599,8 @@ static int rdt_get_tree(struct fs_context *fc)
>>>  out_schemata_free:
>>>  	schemata_list_destroy();
>>>  out_ctx:
>>> +	if (ctx->enable_debug)
>>> +		resctrl_debug = false;
>>>  	if (ctx->enable_mba_mbps)
>>>  		set_mba_sc(false);
>>>  	cdp_disable_all();
>>
>> These changes are a red flag to me. Developers still need to
>> do what patch #4 was aiming to prevent.
> 
> I think you meant patch 5 (x86/resctrl: Unwind the errors inside
> rdt_enable_ctx).
> 
> I can take care of this unwind part in rdt_disable_ctx() and call it here.
> https://lore.kernel.org/lkml/9fd70ef3-ca90-65e3-4746-7d574bdd159b@intel.com/#t
> 
> Hope that is what you meant.

It is. Thank you for reading what I meant to write.

Reinette
  

Patch

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index af234681756e..5a2346d2c561 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -35,7 +35,7 @@  about the feature from resctrl's info directory.
 
 To use the feature mount the file system::
 
- # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl
+ # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps][,debug]] /sys/fs/resctrl
 
 mount options are:
 
@@ -46,6 +46,9 @@  mount options are:
 "mba_MBps":
 	Enable the MBA Software Controller(mba_sc) to specify MBA
 	bandwidth in MBps
+"debug":
+	Make debug files accessible. Available debug files are annotated with
+	"Available only with debug option".
 
 L2 and L3 CDP are controlled separately.
 
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 885b656d1088..5bc18accb887 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -59,6 +59,7 @@  struct rdt_fs_context {
 	bool				enable_cdpl2;
 	bool				enable_cdpl3;
 	bool				enable_mba_mbps;
+	bool				enable_debug;
 };
 
 static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
@@ -296,6 +297,7 @@  struct rdtgroup {
 #define RFTYPE_TOP			BIT(6)
 #define RFTYPE_RES_CACHE		BIT(8)
 #define RFTYPE_RES_MB			BIT(9)
+#define RFTYPE_DEBUG			BIT(10)
 #define RFTYPE_CTRL_INFO		(RFTYPE_INFO | RFTYPE_CTRL)
 #define RFTYPE_MON_INFO			(RFTYPE_INFO | RFTYPE_MON)
 #define RFTYPE_TOP_INFO			(RFTYPE_INFO | RFTYPE_TOP)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e0570bce76a2..058c1dedb2d7 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -56,6 +56,8 @@  static char last_cmd_status_buf[512];
 
 struct dentry *debugfs_resctrl;
 
+static bool resctrl_debug;
+
 void rdt_last_cmd_clear(void)
 {
 	lockdep_assert_held(&rdtgroup_mutex);
@@ -1871,6 +1873,9 @@  static int rdtgroup_add_files(struct kernfs_node *kn, unsigned long fflags)
 
 	lockdep_assert_held(&rdtgroup_mutex);
 
+	if (resctrl_debug)
+		fflags |= RFTYPE_DEBUG;
+
 	for (rft = rfts; rft < rfts + len; rft++) {
 		if (rft->fflags && ((fflags & rft->fflags) == rft->fflags)) {
 			ret = rdtgroup_add_file(kn, rft);
@@ -2399,6 +2404,9 @@  static int rdt_enable_ctx(struct rdt_fs_context *ctx)
 			goto out_cdpl3;
 	}
 
+	if (ctx->enable_debug)
+		resctrl_debug = true;
+
 	return 0;
 
 out_cdpl3:
@@ -2591,6 +2599,8 @@  static int rdt_get_tree(struct fs_context *fc)
 out_schemata_free:
 	schemata_list_destroy();
 out_ctx:
+	if (ctx->enable_debug)
+		resctrl_debug = false;
 	if (ctx->enable_mba_mbps)
 		set_mba_sc(false);
 	cdp_disable_all();
@@ -2608,6 +2618,7 @@  enum rdt_param {
 	Opt_cdp,
 	Opt_cdpl2,
 	Opt_mba_mbps,
+	Opt_debug,
 	nr__rdt_params
 };
 
@@ -2615,6 +2626,7 @@  static const struct fs_parameter_spec rdt_fs_parameters[] = {
 	fsparam_flag("cdp",		Opt_cdp),
 	fsparam_flag("cdpl2",		Opt_cdpl2),
 	fsparam_flag("mba_MBps",	Opt_mba_mbps),
+	fsparam_flag("debug",		Opt_debug),
 	{}
 };
 
@@ -2640,6 +2652,9 @@  static int rdt_parse_param(struct fs_context *fc, struct fs_parameter *param)
 			return -EINVAL;
 		ctx->enable_mba_mbps = true;
 		return 0;
+	case Opt_debug:
+		ctx->enable_debug = true;
+		return 0;
 	}
 
 	return -EINVAL;
@@ -2829,6 +2844,8 @@  static void rdt_kill_sb(struct super_block *sb)
 
 	set_mba_sc(false);
 
+	resctrl_debug = false;
+
 	/*Put everything back to default values. */
 	for_each_alloc_capable_rdt_resource(r)
 		reset_all_ctrls(r);
@@ -3710,6 +3727,9 @@  static int rdtgroup_show_options(struct seq_file *seq, struct kernfs_root *kf)
 	if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl))
 		seq_puts(seq, ",mba_MBps");
 
+	if (resctrl_debug)
+		seq_puts(seq, ",debug");
+
 	return 0;
 }