linux-kernel - Re: [PATCH v3 7/7] x86/resctrl: Add debug files when mounted with debug option

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ac5f5bf8-0fa9-3e93-80ec-bba20a615bfe@amd.com>
Date:   Wed, 22 Mar 2023 10:09:09 -0500
From:   "Moger, Babu" <babu.moger@....com>
To:     Reinette Chatre <reinette.chatre@...el.com>, corbet@....net,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de
Cc:     fenghua.yu@...el.com, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, paulmck@...nel.org, akpm@...ux-foundation.org,
        quic_neeraju@...cinc.com, rdunlap@...radead.org,
        damien.lemoal@...nsource.wdc.com, songmuchun@...edance.com,
        peterz@...radead.org, jpoimboe@...nel.org, pbonzini@...hat.com,
        chang.seok.bae@...el.com, pawan.kumar.gupta@...ux.intel.com,
        jmattson@...gle.com, daniel.sneddon@...ux.intel.com,
        sandipan.das@....com, tony.luck@...el.com, james.morse@....com,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        bagasdotme@...il.com, eranian@...gle.com,
        christophe.leroy@...roup.eu, jarkko@...nel.org,
        adrian.hunter@...el.com, quic_jiles@...cinc.com,
        peternewman@...gle.com
Subject: Re: [PATCH v3 7/7] x86/resctrl: Add debug files when mounted with
 debug option



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