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: <6fb0d504-b8f2-4b16-9d49-b97e41d8a697@intel.com>
Date: Wed, 3 Dec 2025 13:21:56 -0800
From: Reinette Chatre <reinette.chatre@...el.com>
To: "Luck, Tony" <tony.luck@...el.com>
CC: Fenghua Yu <fenghuay@...dia.com>, Maciej Wieczor-Retman
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>, Chen Yu
	<yu.c.chen@...el.com>, <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v14 24/32] x86/resctrl: Add energy/perf choices to rdt
 boot option

Hi Tony,

On 12/3/25 10:04 AM, Luck, Tony wrote:
> On Tue, Dec 02, 2025 at 08:28:56AM -0800, Reinette Chatre wrote:
>>> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
>>> index 46c64419ec10..50c8b4c50790 100644
>>> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
>>> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
>>> @@ -57,12 +57,16 @@ struct pmt_event {
>>>   * struct event_group - Events with the same feature type ("energy" or "perf") and guid.
>>>   * @feature:		Type of events, for example FEATURE_PER_RMID_PERF_TELEM or
>>>   *			FEATURE_PER_RMID_ENERGY_TELEM, in this group.
>>> + * @name:		Name for this group (used by boot rdt= option)
>>
>> This needs a new definition since multiple groups can have the same name now.
> 
> How about this:
> 
>  * @type:               Type (energy or perf) of this group.

I find this to be confusing when considering it together with existing @feature and its
definition that also refers to itself as a "type" using perf and energy terms.

> 
> That covers how different instances have the same string where "name"
> was confusing.
> 
Essentially this is the name used for @feature and for this there is already
pmt_feature_names[]. Is it needed to create a new name? This would mean that the
kernel parameters become "per_rmid_energy_telemetry" and "per_rmid_perf_telemetry" which
is much longer though. The only limiting factor I am aware of is the command line size which
is defined in arch/x86/include/asm/setup.h as 2048. Here I do not know if there are customs on
whether kernel parameters need to be brief or not ... some kernel parameters seem to be quite
verbose while others are cryptic.

The safest may be to stick with the separate names but I am curious about your opinion.

Even so, it seems unnecessary to force each new instance of struct event_group to
set a parameter that is required to be of particular value based on value of event_group::feature.
If not using pmt_feature_names[] then intel_aet.c could have its own private array
that maps pmt_feature_id to "energy" or "perf". I find that doing so would make it obvious
what this property is/should be.

What do you think?

Reinette
	




Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ