Messages in this thread | | | Date | Fri, 7 Jul 2023 14:42:58 -0700 | Subject | Re: [PATCH v5 5/8] x86/resctrl: Introduce "-o debug" mount option | From | Reinette Chatre <> |
| |
Hi Babu,
On 6/1/2023 12:01 PM, Babu Moger wrote: > Add "-o debug" option to mount resctrl filesystem in debug mode. > This option can be used for adding extra files to help debug > resctrl issues.
Please avoid uncertainty in the changelog (re. "can be used"). I think it will help to be more specific if the first and last hunks of patch 8 is merged into this patch, making it clear that the debug mount option is in support of debug files with this changelog written to support that.
> For example, hardware uses CLOSID and RMIDs to control and monitor > resctrl resources. These numbers are not visible to the user. These > details can help to debug issues. Debug option provides a method to > add these files to resctrl.
With the debug file support added here this extra motivation should not be necessary (remaining hunks of patch 8 can be moved to where these files are introduced). > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > Documentation/arch/x86/resctrl.rst | 3 +++ > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 15 +++++++++++++++ > 3 files changed, 19 insertions(+) > > diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst > index 290f01aa3002..afdee4d1f691 100644 > --- a/Documentation/arch/x86/resctrl.rst > +++ b/Documentation/arch/x86/resctrl.rst > @@ -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". >
At the risk of becoming unreadable, the earlier documentation of the mount command should also change.
> 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 c20cd6acb7a3..c07c6517d856 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) > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index baed20b2d788..be91dea5f927 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); > @@ -2368,6 +2370,9 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx) > if (!ret && ctx->enable_mba_mbps) > ret = set_mba_sc(true); > > + if (!ret && ctx->enable_debug) > + resctrl_debug = true; > + > return ret; > }
Looks like unwinding of rdt_enable_ctx() errors is done in the caller, this is not ideal and additions like above cause some error unwinding to be omitted. I cannot see why rdt_enable_ctx() cannot do its own error unwinding. This may be better as a separate patch.
Reinette
| |