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

["Moger, Babu" <bmoger@amd.com>]

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@amd.com>
>> ---
>>   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