Re: [PATCH v13 12/32] x86,fs/resctrl: Support binary fixed point event counters

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Dave,

On 11/11/25 9:22 AM, Dave Martin wrote:
> Hi,
> 
> On Wed, Nov 05, 2025 at 04:09:28PM -0800, Reinette Chatre wrote:
>> Hi Dave and Tony,
>>
>> On 11/5/25 3:31 PM, Luck, Tony wrote:
>>> On Wed, Nov 05, 2025 at 02:42:18PM +0000, Dave Martin wrote:
>>>> On Wed, Oct 29, 2025 at 09:20:55AM -0700, Tony Luck wrote:
>>
>> ...
>>
>>>>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>>>>> index 40b76eaa33d0..f5189b6771a0 100644
>>>>> --- a/fs/resctrl/internal.h
>>>>> +++ b/fs/resctrl/internal.h
>>>>> @@ -62,6 +62,9 @@ static inline struct rdt_fs_context *rdt_fc2context(struct fs_context *fc)
>>>>>   *			Only valid if @evtid is an MBM event.
>>>>>   * @configurable:	true if the event is configurable
>>>>>   * @any_cpu:		true if the event can be read from any CPU
>>>>> + * @is_floating_point:	event values are displayed in floating point format
>>>>
>>>> Nit: Maybe rebrand this as is_fixed_point, or is_fractional, or similar?
>>>>
>>>> The print syntax is just a decimal fraction, and the hardware
>>>> representation is fixed-point.  Nothing floats.
>>>
>>> You are right. I can change from is_floating_point to is_fixed_point.
>>>
>>
>> This is a fs property though, not hardware, and highlights that the value is displayed in
>> floating point format which is the closest resctrl has to establish a "contract" with user
>> space on what format user space can expect when reading the data as backed with a
>> matching update to resctrl.rst for the events that have this hardcoded by the fs.
>> Whether an architecture uses fixed point format or some other mechanism to determine the
>> value eventually exposed to user space is unique to the architecture. 
> 
> Sure, getting the docmuentation right is the most important thing,
> while the internal name for this property is not ABI.
> 
> (I don't strongly object to "is_floating_point", even if we expose this
> in the filesystem, so long as we document carefully what it means.)

Highlighting the member name and description in fs/resctrl/internal.h: 
	@is_floating_point:	event values are displayed in floating point format

I consider it important that the description highlights that the event will be displayed to
user space as floating point. struct mon_evt that contains this member is internal to resctrl fs
and there is no helper available to arch with which @is_floating_point can be changed since
this is a contract with user space. I find that having the member name match that description
and contract easier to read.

The documentation (resctrl.rst) is updated in patch #32 with below to make this clear:

	"core energy" reports a floating point number for the energy (in Joules) ...
	...
	"activity" also reports a floating point value (in Farads).

I agree that internal names are not ABI and this is evident with the only internal
connection to a value displayed as floating point being an internal fixed point fraction
number. This can change any time. We have to draw the line somewhere to make it clear
how resctrl interacts with user space and I find the event's display property to be
appropriate for this.

Reinette