Re: [PATCH v4 13/31] fs/resctrl: Add support for additional monitor event display formats

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

Hi Tony,

shortlog nit: "fs/resctrl: Support additional monitor event display formats"

On 4/28/25 5:33 PM, Tony Luck wrote:
> Resctrl was written with the assumption that all monitor events
> can be displayed as unsigned decimal integers.
> 
> Some telemetry events provide greater precision where architecture code
> uses a fixed point format with 18 binary places.
> 
> Add a "display_format" field to struct mon_evt which can specify
> that the value for the event be displayed as an integer for legacy
> events, or as a floating point value with six decimal places converted
> from the fixed point format received from architecture code.

There was no discussion on this during the previous version.
While this version addresses the issue of architecture changing the
format it does not address the issue of how to handle different
architecture formats. With this change any architecture that may
want to support any of these events will be required to translate
whatever format it uses into the one Intel uses to be translated
again into format for user space. Do you think this is reasonable? 

Alternatively, resctrl could add additional file that contains the
format so that if an architecture in the future needs to present data
differently, an interface will exist to guide userspace how to parse it.
Creation of such user interface cannot be delayed until the time
it is needed since then these formats would be ABI.

> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  include/linux/resctrl_types.h |  5 +++++
>  fs/resctrl/internal.h         |  2 ++
>  fs/resctrl/ctrlmondata.c      | 24 +++++++++++++++++++++++-
>  fs/resctrl/monitor.c          | 21 ++++++++++++---------
>  4 files changed, 42 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/resctrl_types.h b/include/linux/resctrl_types.h
> index 5ef14a24008c..6245034f6c76 100644
> --- a/include/linux/resctrl_types.h
> +++ b/include/linux/resctrl_types.h

This needs to be internal to resctrl fs.
resctrl_types.h should only contain the types required in asm/resctrl.h

> @@ -50,4 +50,9 @@ enum resctrl_event_id {
>  #define QOS_NUM_MBM_EVENTS	(QOS_L3_MBM_LOCAL_EVENT_ID - QOS_L3_MBM_TOTAL_EVENT_ID + 1)
>  #define MBM_EVENT_IDX(evt)	((evt) - QOS_L3_MBM_TOTAL_EVENT_ID)
>  
> +/* Event value display formats */

Please add details about what each format means (how it should
be interpreted).

Reinette