| 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
| ||
|
Message-ID: <5ab0a5c4-a79c-4bfc-84fd-3eeb0b9aeb53@intel.com>
Date: Fri, 25 Jul 2025 16:34:42 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, 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>, Anil Keshavamurthy <anil.s.keshavamurthy@...el.com>,
Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<patches@...ts.linux.dev>
Subject: Re: [PATCH v7 14/31] x86,fs/resctrl: Support binary fixed point event
counters
Hi Tony,
On 7/11/25 4:53 PM, Tony Luck wrote:
> Resctrl was written with the assumption that all monitor events can be
> displayed as unsigned decimal integers.
"resctrl assumes that ..."
>
> Hardware architecture counters may provide some telemetry events with
> greater precision where the event is not a simple count, but is a
> measurement of some sort (e.g. Joules for energy consumed).
>
> Add a new argument to resctrl_enable_mon_event() for architecture code
> to inform the file system that the value for a counter is a fixed-point
> value with a specific number of binary places. The file system will
> only allow architecture to use floating point format on events that it
"The file system will only allow ..." -> "Only allow ..."
> marked with mon_evt::is_floating_point.
>
> Fixed point values are displayed with values rounded to an appropriate
"Display fixed point values ..."
(please review all changelogs to ensure tip requirements are met)
> number of decimal places for the precision of the number of binary places
> provided. In general one extra decimal place is added for every three
Needs imperative. For example:
"Add one extra decimal place for every three additional binary places,
except for low precision binary values where exact representation is possible"?
> additional binary places. There are some exceptions for low precision
> binary values where exact representation is possible:
>
> 1 binary place is 0.0 or 0.5. => 1 decimal place
> 2 binary places is 0.0, 0.25, 0.5, 0.75 => 2 decimal places
> 3 binary places is 0.0, 0.125, etc. => 3 decimal places
>
> Signed-off-by: Tony Luck <tony.luck@...el.com>
> ---
> include/linux/resctrl.h | 4 +-
> fs/resctrl/internal.h | 5 ++
> arch/x86/kernel/cpu/resctrl/core.c | 6 +-
> fs/resctrl/ctrlmondata.c | 88 ++++++++++++++++++++++++++++++
> fs/resctrl/monitor.c | 10 +++-
> 5 files changed, 107 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 17a21f193a3d..e9a1cabfc724 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -379,7 +379,9 @@ u32 resctrl_arch_get_num_closid(struct rdt_resource *r);
> u32 resctrl_arch_system_num_rmid_idx(void);
> int resctrl_arch_update_domains(struct rdt_resource *r, u32 closid);
>
> -void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu);
> +#define MAX_BINARY_BITS 27
> +
> +void resctrl_enable_mon_event(enum resctrl_event_id eventid, bool any_cpu, unsigned int binary_bits);
This declartion is now over 100 columns and will anyway be split across two lines
in patch #21. That split can be done here to eliminate one checkpatch.pl complaint.
>
> bool resctrl_is_mon_event_enabled(enum resctrl_event_id eventid);
>
...
> diff --git a/fs/resctrl/ctrlmondata.c b/fs/resctrl/ctrlmondata.c
> index 2e65fddc3408..71d61c96c2b8 100644
> --- a/fs/resctrl/ctrlmondata.c
> +++ b/fs/resctrl/ctrlmondata.c
> @@ -17,6 +17,7 @@
>
> #include <linux/cpu.h>
> #include <linux/kernfs.h>
> +#include <linux/math.h>
> #include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/tick.h>
> @@ -590,6 +591,91 @@ void mon_event_read(struct rmid_read *rr, struct rdt_resource *r,
> resctrl_arch_mon_ctx_free(r, evt->evtid, rr->arch_mon_ctx);
> }
>
> +/*
> + * Decimal place precision to use for each number of fixed-point
> + * binary bits.
> + */
> +static unsigned int decplaces[MAX_BINARY_BITS + 1] = {
> + [1] = 1,
> + [2] = 2,
> + [3] = 3,
> + [4] = 3,
> + [5] = 3,
> + [6] = 3,
> + [7] = 3,
> + [8] = 3,
> + [9] = 3,
> + [10] = 4,
> + [11] = 4,
> + [12] = 4,
> + [13] = 5,
> + [14] = 5,
> + [15] = 5,
> + [16] = 6,
> + [17] = 6,
> + [18] = 6,
> + [19] = 7,
> + [20] = 7,
> + [21] = 7,
> + [22] = 8,
> + [23] = 8,
> + [24] = 8,
> + [25] = 9,
> + [26] = 9,
> + [27] = 9
> +};
> +
> +static void print_event_value(struct seq_file *m, unsigned int binary_bits, u64 val)
> +{
> + unsigned long long frac;
> + char buf[10];
> +
> + if (!binary_bits) {
> + seq_printf(m, "%llu.0\n", val);
> + return;
> + }
> +
> + /* Mask off the integer part of the fixed-point value. */
> + frac = val & GENMASK_ULL(binary_bits, 0);
> +
> + /*
> + * Multiply by 10^{desired decimal places}. The
> + * integer part of the fixed point value is now
> + * almost what is needed.
Please expand comments to 80 columns.
> + */
> + frac *= int_pow(10ull, decplaces[binary_bits]);
> +
> + /*
> + * Round to nearest by adding a value that
> + * would be a "1" in the binary_bit + 1 place.
"binary_bit" -> "binary_bits"?
> + * Integer part of fixed point value is now
> + * the needed value.
> + */
> + frac += 1ull << (binary_bits - 1);
> +
> + /*
> + * Extract the integer part of the value. This
> + * is the decimal representation of the original
> + * fixed-point fractional value.
> + */
> + frac >>= binary_bits;
> +
> + /*
> + * "frac" is now in the range [0 .. 10^decplaces).
> + * I.e. string representation will fit into
> + * chosemn number of decimal places.
chosemn -> chosen
> + */
> + snprintf(buf, sizeof(buf), "%0*llu", decplaces[binary_bits], frac);
> +
> + /* Trim trailing zeroes */
> + for (int i = decplaces[binary_bits] - 1; i > 0; i--) {
> + if (buf[i] != '0')
> + break;
> + buf[i] = '\0';
> + }
> + seq_printf(m, "%llu.%s\n", val >> binary_bits, buf);
> +}
> +
> int rdtgroup_mondata_show(struct seq_file *m, void *arg)
> {
> struct kernfs_open_file *of = m->private;
> @@ -666,6 +752,8 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
> seq_puts(m, "Error\n");
> else if (rr.err == -EINVAL)
> seq_puts(m, "Unavailable\n");
> + else if (evt->is_floating_point)
> + print_event_value(m, evt->binary_bits, rr.val);
> else
> seq_printf(m, "%llu\n", rr.val);
>
Reinette
Powered by blists - more mailing lists