linux-kernel - Re: [PATCH v8 5/7] x86/resctrl: Add "mba_MBps_event" file to ctrl

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: <c748f9d5-ef10-499e-bde5-1bce20a44d2d@intel.com>
Date: Tue, 12 Nov 2024 16:20:14 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>, "Yu, Fenghua" <fenghua.yu@...el.com>,
	Peter Newman <peternewman@...gle.com>, Jonathan Corbet <corbet@....net>,
	Shuah Khan <skhan@...uxfoundation.org>, "x86@...nel.org" <x86@...nel.org>
CC: James Morse <james.morse@....com>, Jamie Iles <quic_jiles@...cinc.com>,
	Babu Moger <babu.moger@....com>, Randy Dunlap <rdunlap@...radead.org>,
	"Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"patches@...ts.linux.dev" <patches@...ts.linux.dev>
Subject: Re: [PATCH v8 5/7] x86/resctrl: Add "mba_MBps_event" file to ctrl_mon
 directories

Hi Tony,

On 11/12/24 3:42 PM, Luck, Tony wrote:
>>> +static int set_mba_sc(bool mba_sc)
>>> +{
>>> +   struct rftype *rft;
>>> +
>>> +   rft = rdtgroup_get_rftype_by_name("mba_MBps_event");
>>> +   if (rft)
>>> +           rft->fflags = enable ? RFTYPE_CTRL_BASE : 0;
>>
>> I think this sets this file to be created for all CTRL groups, even when not supporting
>> monitoring?
> 
> No. The calling sequence is:
> 
> rdt_get_tree()
>     rdt_enable_ctx()
> 
>         if (ctx->enable_mba_mbps) {
>                 ret = set_mba_sc(true);
>                 if (ret)
>                         goto out_cdpl3;
>         }
> 
> So set_mba_sc() is only called when the mba_MBps mount option has been used. So
> mba_mbps_event_init() isn't called.
> 
> Note that on unmount of the resctrl file system rdt_kill_sb() calls rdt_disable_ctx()
> which calls set_mba_sc(false) which will clear rft->fflags on systems which support
> mba_MBps.

It sounds as though you are saying that setting the wrong flags are ok as long as it is
done in some specific calling sequence. Is this correct? Relying on calling sequence
instead of correct flags requires more in-depth knowledge of code flows and makes code
harder to maintain.
Why not just make this "RFTYPE_CTRL_BASE | RFTYPE_MON_BASE" to make it clear that the file
applies to CTRL_MON groups? What am I missing?

Reinette