Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option

[Moger, Babu]

Hi Fenghua,

On 2/16/2023 7:42 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:47, Babu Moger wrote:
> > Add -o debug option to mount resctrl filesystem in debug mode. Debug
> > option adds the files for debug purposes.
> >
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> >   Documentation/x86/resctrl.rst          |    2 ++
> >   arch/x86/kernel/cpu/resctrl/internal.h |    1 +
> >   arch/x86/kernel/cpu/resctrl/rdtgroup.c |    7 +++++++
> >   3 files changed, 10 insertions(+)
> >
> > diff --git a/Documentation/x86/resctrl.rst 
> > b/Documentation/x86/resctrl.rst
> > index 58b76fc75cb7..2c013c5d45fd 100644
> > --- a/Documentation/x86/resctrl.rst
> > +++ b/Documentation/x86/resctrl.rst
> > @@ -46,6 +46,8 @@ mount options are:
> >   "mba_MBps":
> >       Enable the MBA Software Controller(mba_sc) to specify MBA
> >       bandwidth in MBps
> > +"debug":
> > +    Lists the debug files in resctrl interface
> >     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 6767c85b9699..35a9ee343fe0 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                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 18d458a3cba6..9b7813aa6baf 100644
> > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> > @@ -2555,6 +2555,7 @@ enum rdt_param {
> >       Opt_cdp,
> >       Opt_cdpl2,
> >       Opt_mba_mbps,
> > +    Opt_debug,
> >       nr__rdt_params
> >   };
> >   @@ -2562,6 +2563,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),
> >       {}
> >   };
> >   @@ -2587,6 +2589,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->debug = true;
> > +        return 0;
> >       }
> >         return -EINVAL;
> > @@ -3525,6 +3530,8 @@ 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");
> >   +    seq_puts(seq, ",debug");
>
> Need to add a check here otherwise ",debug" will be always shown 
> regardless "-o debug" is given or not:
> +    if (ctx->debug)
> +        seq_puts(seq, ",debug");
>
> But I don't know a good way to get ctx->debug in this function yet. I 
> think somehow ctx can be retrieved from kf but not sure.

Yes. Make sense. May be i will have to save it in rdt_hw_resource then I 
can add that check.

Thanks

Babu


> > +
> >       return 0;
> >   }
> Thanks.
>
> -Fenghua