[PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period

mark.barnett@arm.com posted 5 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by mark.barnett@arm.com 11 months, 1 week ago
From: Ben Gainey <ben.gainey@arm.com>

This change modifies the core perf overflow handler, adding some small
random jitter to each sample period whenever an event switches between the
two alternate sample periods. A new flag is added to perf_event_attr to
opt into this behaviour.

This change follows the discussion in [1], where it is recognized that it
may be possible for certain patterns of execution to end up with biased
results.

[1] https://lore.kernel.org/linux-perf-users/Zc24eLqZycmIg3d2@tassilo/

Signed-off-by: Ben Gainey <ben.gainey@arm.com>
Signed-off-by: Mark Barnett <mark.barnett@arm.com>
---
 include/uapi/linux/perf_event.h | 7 ++++++-
 kernel/events/core.c            | 9 ++++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 499a8673df8e..c0076ce8f80a 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -461,7 +461,12 @@ struct perf_event_attr {
 				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
 				remove_on_exec :  1, /* event is removed from task on exec */
 				sigtrap        :  1, /* send synchronous SIGTRAP on event */
-				__reserved_1   : 26;
+				/*
+				 * Add a limited amount of jitter on each alternate period, where
+				 * the jitter is between [0, (2<<jitter_alt_period) - 1]
+				 */
+				jitter_alt_period : 3,
+				__reserved_1   : 23;
 
 	union {
 		__u32		wakeup_events;	  /* wakeup every n events */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7ec8ec6ba7ef..be271e21cd06 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -15,6 +15,7 @@
 #include <linux/idr.h>
 #include <linux/file.h>
 #include <linux/poll.h>
+#include <linux/random.h>
 #include <linux/slab.h>
 #include <linux/hash.h>
 #include <linux/tick.h>
@@ -9922,7 +9923,10 @@ static int __perf_event_overflow(struct perf_event *event,
 	if (event->attr.alt_sample_period) {
 		bool using_alt = hwc->using_alt_sample_period;
 		u64 sample_period = (using_alt ? event->attr.sample_period
-					       : event->attr.alt_sample_period);
+					       : event->attr.alt_sample_period)
+				  + (event->attr.jitter_alt_period
+					? get_random_u32_below(2 << event->attr.jitter_alt_period)
+					: 0);
 
 		hwc->sample_period = sample_period;
 		hwc->using_alt_sample_period = !using_alt;
@@ -12849,6 +12853,9 @@ SYSCALL_DEFINE5(perf_event_open,
 		}
 	}
 
+	if (attr.jitter_alt_period && !attr.alt_sample_period)
+		return -EINVAL;
+
 	/* Only privileged users can get physical addresses */
 	if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR)) {
 		err = perf_allow_kernel(&attr);
-- 
2.43.0
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Fri, Mar 07, 2025 at 08:22:45PM +0000, mark.barnett@arm.com wrote:
> From: Ben Gainey <ben.gainey@arm.com>
> 
> This change modifies the core perf overflow handler, adding some small
> random jitter to each sample period whenever an event switches between the
> two alternate sample periods. A new flag is added to perf_event_attr to
> opt into this behaviour.
> 
> This change follows the discussion in [1], where it is recognized that it
> may be possible for certain patterns of execution to end up with biased
> results.
> 
> [1] https://lore.kernel.org/linux-perf-users/Zc24eLqZycmIg3d2@tassilo/
> 
> Signed-off-by: Ben Gainey <ben.gainey@arm.com>
> Signed-off-by: Mark Barnett <mark.barnett@arm.com>
> ---
>  include/uapi/linux/perf_event.h | 7 ++++++-
>  kernel/events/core.c            | 9 ++++++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 499a8673df8e..c0076ce8f80a 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -461,7 +461,12 @@ struct perf_event_attr {
>  				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
>  				remove_on_exec :  1, /* event is removed from task on exec */
>  				sigtrap        :  1, /* send synchronous SIGTRAP on event */
> -				__reserved_1   : 26;
> +				/*
> +				 * Add a limited amount of jitter on each alternate period, where
> +				 * the jitter is between [0, (2<<jitter_alt_period) - 1]
> +				 */
> +				jitter_alt_period : 3,
> +				__reserved_1   : 23;

So; I've been thinking about this interface.

I think I prefer you keep the existing sample_period/sample_freq working
as is and simply modulate with random and high-freq components.

A very little like so..

I've made the hf_sample_period 32bit since I figured that ought to be
enough -- you're aiming at very short periods after all. But there's
enough unused bits left.

So this has sample_period or sample_freq compute hwc->sample_period_base
which is first modified with random such that the average is exactly
sample_period_base (assuming a flat distribution).

This means that sample_period_base is still the right number to use for
computing freq based things. Additionally, have the 'extra' interrupt
ignored for adaptive period crud.

Also, someone needs to consider the eBPF hook and what to do with it.
I've kept the ordering as per this series, but I suspect it's wrong and
we want this before the BPF hook. Please think about this and explicitly
mention this in the next series.

Anyway, very much a sketch of things, incomplete and not been near a
compiler.



---
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 76f4265efee9..c5dd6442e96f 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -229,6 +229,10 @@ struct hw_perf_event {
 #define PERF_HES_UPTODATE	0x02 /* event->count up-to-date */
 #define PERF_HES_ARCH		0x04
 
+#define PERF_HES_HF_ON		0x10 /* using high-fred sampling */
+#define PERF_HES_HF_SAMPLE	0x20
+#define PERF_HES_HF_RAND	0x40
+
 	int				state;
 
 	/*
@@ -241,6 +245,7 @@ struct hw_perf_event {
 	 * The period to start the next sample with.
 	 */
 	u64				sample_period;
+	u64				sample_period_base;
 
 	union {
 		struct { /* Sampling */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 0524d541d4e3..8dbe027f93f1 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -379,6 +379,7 @@ enum perf_event_read_format {
 #define PERF_ATTR_SIZE_VER6	120	/* add: aux_sample_size */
 #define PERF_ATTR_SIZE_VER7	128	/* add: sig_data */
 #define PERF_ATTR_SIZE_VER8	136	/* add: config3 */
+#define PERF_ATTR_SIZE_VER9	144	/* add: hf_sample */
 
 /*
  * Hardware event_id to monitor via a performance monitoring event:
@@ -531,6 +532,14 @@ struct perf_event_attr {
 	__u64	sig_data;
 
 	__u64	config3; /* extension of config2 */
+	union {
+		__u64 hf_sample;
+		struct {
+			__u64 hf_sample_period : 32,
+			      hf_sample_rand   :  4,
+			      __reserved_4     : 28;
+		};
+	};
 };
 
 /*
diff --git a/kernel/events/core.c b/kernel/events/core.c
index b87a5ac42ce2..e5a93edf3b5f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8,6 +8,7 @@
  *  Copyright  ©  2009 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
  */
 
+#include "linux/random.h"
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/cpu.h>
@@ -55,6 +56,7 @@
 #include <linux/pgtable.h>
 #include <linux/buildid.h>
 #include <linux/task_work.h>
+#include <linux/prandom.h>
 
 #include "internal.h"
 
@@ -443,6 +445,8 @@ static cpumask_var_t perf_online_pkg_mask;
 static cpumask_var_t perf_online_sys_mask;
 static struct kmem_cache *perf_event_cache;
 
+static struct rnd_state perf_rand;
+
 /*
  * perf event paranoia level:
  *  -1 - not paranoid at all
@@ -4233,19 +4237,19 @@ static void perf_adjust_period(struct perf_event *event, u64 nsec, u64 count, bo
 
 	period = perf_calculate_period(event, nsec, count);
 
-	delta = (s64)(period - hwc->sample_period);
+	delta = (s64)(period - hwc->sample_period_base);
 	if (delta >= 0)
 		delta += 7;
 	else
 		delta -= 7;
 	delta /= 8; /* low pass filter */
 
-	sample_period = hwc->sample_period + delta;
+	sample_period = hwc->sample_period_base + delta;
 
 	if (!sample_period)
 		sample_period = 1;
 
-	hwc->sample_period = sample_period;
+	hwc->sample_period_base = sample_period;
 
 	if (local64_read(&hwc->period_left) > 8*sample_period) {
 		if (disable)
@@ -4490,6 +4494,8 @@ void perf_event_task_tick(void)
 	if (ctx)
 		perf_adjust_freq_unthr_context(ctx, !!throttled);
 	rcu_read_unlock();
+
+	prandom_seed_state(&perf_rand, get_random_u64());
 }
 
 static int event_enable_on_exec(struct perf_event *event,
@@ -9979,6 +9985,8 @@ static int __perf_event_overflow(struct perf_event *event,
 				 int throttle, struct perf_sample_data *data,
 				 struct pt_regs *regs)
 {
+	struct hw_perf_event *hwc = &event->hw;
+	u64 sample_period;
 	int events = atomic_read(&event->event_limit);
 	int ret = 0;
 
@@ -9989,15 +9997,50 @@ static int __perf_event_overflow(struct perf_event *event,
 	if (unlikely(!is_sampling_event(event)))
 		return 0;
 
-	ret = __perf_event_account_interrupt(event, throttle);
+	/*
+	 * High Freq samples are injected inside the larger period:
+	 *
+	 *   |------------|-|------------|-|
+	 *   P0          HF P1          HF
+	 *
+	 * By ignoring the HF samples, we measure the actual period.
+	 */
+	if (!(hwc->state & PERF_HES_HF_SAMPLE))
+		ret = __perf_event_account_interrupt(event, throttle);
 
 	if (event->attr.aux_pause)
 		perf_event_aux_pause(event->aux_event, true);
 
+	/* XXX interaction between HF samples and BPF */
 	if (event->prog && event->prog->type == BPF_PROG_TYPE_PERF_EVENT &&
 	    !bpf_overflow_handler(event, data, regs))
 		goto out;
 
+	sample_period = hwc->sample_period_base;
+	if (hwc->state & PERF_HES_HF_RAND) {
+		u64 rand = 1 << event->attr.hf_sample_rand;
+		sample_period -= rand / 2;
+		sample_period += prandom_u32_state(&perf_rand) & (rand - 1);
+	}
+	if (hwc->state & PERF_HES_HF_ON) {
+		u64 hf_sample_period = event->attr.hf_sample_period;
+
+		if (sample_period < hf_sample_period) {
+			hwc->state &= ~PERF_HES_HF_ON;
+			goto set_period;
+		}
+
+		if (!(hwc->state & PERF_HES_HF_SAMPLE)) {
+			hwc->sample_period -= hf_sample_period;
+			hwc->state |= PERF_HES_HF_SAMPLE;
+		} else {
+			hwc->sample_period = hf_sample_period;
+			hwc->state &= ~PERF_HES_HF_SAMPLE;
+		}
+	}
+set_period:
+	hwc->sample_period = sample_period;
+
 	/*
 	 * XXX event_limit might not quite work as expected on inherited
 	 * events
@@ -12458,8 +12501,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 
 	hwc = &event->hw;
 	hwc->sample_period = attr->sample_period;
-	if (attr->freq && attr->sample_freq)
+	hwc->sample_period_base = attr->sample_period;
+	if (attr->freq && attr->sample_freq) {
 		hwc->sample_period = 1;
+		hwc->sample_period_base = 1;
+	}
 	hwc->last_period = hwc->sample_period;
 
 	local64_set(&hwc->period_left, hwc->sample_period);
@@ -13824,6 +13870,7 @@ inherit_event(struct perf_event *parent_event,
 		struct hw_perf_event *hwc = &child_event->hw;
 
 		hwc->sample_period = sample_period;
+		hwc->sample_period_base = sample_period;
 		hwc->last_period   = sample_period;
 
 		local64_set(&hwc->period_left, sample_period);
@@ -14279,6 +14326,8 @@ void __init perf_event_init(void)
 {
 	int ret;
 
+	prandom_seed_state(&perf_rand, get_random_u64());
+
 	idr_init(&pmu_idr);
 
 	perf_event_init_all_cpus();
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Mark Barnett 11 months ago
On 3/11/25 11:31, Peter Zijlstra wrote:
> On Fri, Mar 07, 2025 at 08:22:45PM +0000, mark.barnett@arm.com wrote:
>> From: Ben Gainey <ben.gainey@arm.com>
>>
>> This change modifies the core perf overflow handler, adding some small
>> random jitter to each sample period whenever an event switches between the
>> two alternate sample periods. A new flag is added to perf_event_attr to
>> opt into this behaviour.
>>
>> This change follows the discussion in [1], where it is recognized that it
>> may be possible for certain patterns of execution to end up with biased
>> results.
>>
>> [1] https://lore.kernel.org/linux-perf-users/Zc24eLqZycmIg3d2@tassilo/
>>
>> Signed-off-by: Ben Gainey <ben.gainey@arm.com>
>> Signed-off-by: Mark Barnett <mark.barnett@arm.com>
>> ---
>>   include/uapi/linux/perf_event.h | 7 ++++++-
>>   kernel/events/core.c            | 9 ++++++++-
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>> index 499a8673df8e..c0076ce8f80a 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -461,7 +461,12 @@ struct perf_event_attr {
>>   				inherit_thread :  1, /* children only inherit if cloned with CLONE_THREAD */
>>   				remove_on_exec :  1, /* event is removed from task on exec */
>>   				sigtrap        :  1, /* send synchronous SIGTRAP on event */
>> -				__reserved_1   : 26;
>> +				/*
>> +				 * Add a limited amount of jitter on each alternate period, where
>> +				 * the jitter is between [0, (2<<jitter_alt_period) - 1]
>> +				 */
>> +				jitter_alt_period : 3,
>> +				__reserved_1   : 23;
> 
> So; I've been thinking about this interface.
> 
> I think I prefer you keep the existing sample_period/sample_freq working
> as is and simply modulate with random and high-freq components.
> 
> A very little like so..
> 
> I've made the hf_sample_period 32bit since I figured that ought to be
> enough -- you're aiming at very short periods after all. But there's
> enough unused bits left.
> 
> So this has sample_period or sample_freq compute hwc->sample_period_base
> which is first modified with random such that the average is exactly
> sample_period_base (assuming a flat distribution).
> 
> This means that sample_period_base is still the right number to use for
> computing freq based things. Additionally, have the 'extra' interrupt
> ignored for adaptive period crud.
> 
> Also, someone needs to consider the eBPF hook and what to do with it.
> I've kept the ordering as per this series, but I suspect it's wrong and
> we want this before the BPF hook. Please think about this and explicitly
> mention this in the next series.
> 
> Anyway, very much a sketch of things, incomplete and not been near a
> compiler.
> 
> 

Thanks, Peter!

OK, I see what you mean. Packing the fields into hf_sample makes sense. 
I'll have a look at the eBPF hook and see if we need to do anything. The 
sample period is always stored in perf_sample_data so it's technically 
possible for eBPF programs to identify the high-frequency ones, but it's 
not a great API. Maybe we should have an explicit flag.

I have one question about interrupt accounting, below...

> 
> ---
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 76f4265efee9..c5dd6442e96f 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -229,6 +229,10 @@ struct hw_perf_event {
>   #define PERF_HES_UPTODATE	0x02 /* event->count up-to-date */
>   #define PERF_HES_ARCH		0x04
>   
> +#define PERF_HES_HF_ON		0x10 /* using high-fred sampling */
> +#define PERF_HES_HF_SAMPLE	0x20
> +#define PERF_HES_HF_RAND	0x40
> +
>   	int				state;
>   
>   	/*
> @@ -241,6 +245,7 @@ struct hw_perf_event {
>   	 * The period to start the next sample with.
>   	 */
>   	u64				sample_period;
> +	u64				sample_period_base;
>   
>   	union {
>   		struct { /* Sampling */
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 0524d541d4e3..8dbe027f93f1 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -379,6 +379,7 @@ enum perf_event_read_format {
>   #define PERF_ATTR_SIZE_VER6	120	/* add: aux_sample_size */
>   #define PERF_ATTR_SIZE_VER7	128	/* add: sig_data */
>   #define PERF_ATTR_SIZE_VER8	136	/* add: config3 */
> +#define PERF_ATTR_SIZE_VER9	144	/* add: hf_sample */
>   
>   /*
>    * Hardware event_id to monitor via a performance monitoring event:
> @@ -531,6 +532,14 @@ struct perf_event_attr {
>   	__u64	sig_data;
>   
>   	__u64	config3; /* extension of config2 */
> +	union {
> +		__u64 hf_sample;
> +		struct {
> +			__u64 hf_sample_period : 32,
> +			      hf_sample_rand   :  4,
> +			      __reserved_4     : 28;
> +		};
> +	};
>   };
>   
>   /*
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index b87a5ac42ce2..e5a93edf3b5f 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8,6 +8,7 @@
>    *  Copyright  ©  2009 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
>    */
>   
> +#include "linux/random.h"
>   #include <linux/fs.h>
>   #include <linux/mm.h>
>   #include <linux/cpu.h>
> @@ -55,6 +56,7 @@
>   #include <linux/pgtable.h>
>   #include <linux/buildid.h>
>   #include <linux/task_work.h>
> +#include <linux/prandom.h>
>   
>   #include "internal.h"
>   
> @@ -443,6 +445,8 @@ static cpumask_var_t perf_online_pkg_mask;
>   static cpumask_var_t perf_online_sys_mask;
>   static struct kmem_cache *perf_event_cache;
>   
> +static struct rnd_state perf_rand;
> +
>   /*
>    * perf event paranoia level:
>    *  -1 - not paranoid at all
> @@ -4233,19 +4237,19 @@ static void perf_adjust_period(struct perf_event *event, u64 nsec, u64 count, bo
>   
>   	period = perf_calculate_period(event, nsec, count);
>   
> -	delta = (s64)(period - hwc->sample_period);
> +	delta = (s64)(period - hwc->sample_period_base);
>   	if (delta >= 0)
>   		delta += 7;
>   	else
>   		delta -= 7;
>   	delta /= 8; /* low pass filter */
>   
> -	sample_period = hwc->sample_period + delta;
> +	sample_period = hwc->sample_period_base + delta;
>   
>   	if (!sample_period)
>   		sample_period = 1;
>   
> -	hwc->sample_period = sample_period;
> +	hwc->sample_period_base = sample_period;
>   
>   	if (local64_read(&hwc->period_left) > 8*sample_period) {
>   		if (disable)
> @@ -4490,6 +4494,8 @@ void perf_event_task_tick(void)
>   	if (ctx)
>   		perf_adjust_freq_unthr_context(ctx, !!throttled);
>   	rcu_read_unlock();
> +
> +	prandom_seed_state(&perf_rand, get_random_u64());
>   }
>   
>   static int event_enable_on_exec(struct perf_event *event,
> @@ -9979,6 +9985,8 @@ static int __perf_event_overflow(struct perf_event *event,
>   				 int throttle, struct perf_sample_data *data,
>   				 struct pt_regs *regs)
>   {
> +	struct hw_perf_event *hwc = &event->hw;
> +	u64 sample_period;
>   	int events = atomic_read(&event->event_limit);
>   	int ret = 0;
>   
> @@ -9989,15 +9997,50 @@ static int __perf_event_overflow(struct perf_event *event,
>   	if (unlikely(!is_sampling_event(event)))
>   		return 0;
>   
> -	ret = __perf_event_account_interrupt(event, throttle);
> +	/*
> +	 * High Freq samples are injected inside the larger period:
> +	 *
> +	 *   |------------|-|------------|-|
> +	 *   P0          HF P1          HF
> +	 *
> +	 * By ignoring the HF samples, we measure the actual period.
> +	 */
> +	if (!(hwc->state & PERF_HES_HF_SAMPLE))
> +		ret = __perf_event_account_interrupt(event, throttle);
>   

The high-frequency samples should still contribute to interrupt 
accounting/throttling, right? We'd just need to put guards around the 
adaptive period stuff so that HF samples don't contribute to the 
frequency training.

>   	if (event->attr.aux_pause)
>   		perf_event_aux_pause(event->aux_event, true);
>   
> +	/* XXX interaction between HF samples and BPF */
>   	if (event->prog && event->prog->type == BPF_PROG_TYPE_PERF_EVENT &&
>   	    !bpf_overflow_handler(event, data, regs))
>   		goto out;
>   
> +	sample_period = hwc->sample_period_base;
> +	if (hwc->state & PERF_HES_HF_RAND) {
> +		u64 rand = 1 << event->attr.hf_sample_rand;
> +		sample_period -= rand / 2;
> +		sample_period += prandom_u32_state(&perf_rand) & (rand - 1);
> +	}
> +	if (hwc->state & PERF_HES_HF_ON) {
> +		u64 hf_sample_period = event->attr.hf_sample_period;
> +
> +		if (sample_period < hf_sample_period) {
> +			hwc->state &= ~PERF_HES_HF_ON;
> +			goto set_period;
> +		}
> +
> +		if (!(hwc->state & PERF_HES_HF_SAMPLE)) {
> +			hwc->sample_period -= hf_sample_period;
> +			hwc->state |= PERF_HES_HF_SAMPLE;
> +		} else {
> +			hwc->sample_period = hf_sample_period;
> +			hwc->state &= ~PERF_HES_HF_SAMPLE;
> +		}
> +	}
> +set_period:
> +	hwc->sample_period = sample_period;
> +
>   	/*
>   	 * XXX event_limit might not quite work as expected on inherited
>   	 * events
> @@ -12458,8 +12501,11 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
>   
>   	hwc = &event->hw;
>   	hwc->sample_period = attr->sample_period;
> -	if (attr->freq && attr->sample_freq)
> +	hwc->sample_period_base = attr->sample_period;
> +	if (attr->freq && attr->sample_freq) {
>   		hwc->sample_period = 1;
> +		hwc->sample_period_base = 1;
> +	}
>   	hwc->last_period = hwc->sample_period;
>   
>   	local64_set(&hwc->period_left, hwc->sample_period);
> @@ -13824,6 +13870,7 @@ inherit_event(struct perf_event *parent_event,
>   		struct hw_perf_event *hwc = &child_event->hw;
>   
>   		hwc->sample_period = sample_period;
> +		hwc->sample_period_base = sample_period;
>   		hwc->last_period   = sample_period;
>   
>   		local64_set(&hwc->period_left, sample_period);
> @@ -14279,6 +14326,8 @@ void __init perf_event_init(void)
>   {
>   	int ret;
>   
> +	prandom_seed_state(&perf_rand, get_random_u64());
> +
>   	idr_init(&pmu_idr);
>   
>   	perf_event_init_all_cpus();
> 

Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Tue, Mar 11, 2025 at 05:22:16PM +0000, Mark Barnett wrote:

> > @@ -9979,6 +9985,8 @@ static int __perf_event_overflow(struct perf_event *event,
> >   				 int throttle, struct perf_sample_data *data,
> >   				 struct pt_regs *regs)
> >   {
> > +	struct hw_perf_event *hwc = &event->hw;
> > +	u64 sample_period;
> >   	int events = atomic_read(&event->event_limit);
> >   	int ret = 0;
> > @@ -9989,15 +9997,50 @@ static int __perf_event_overflow(struct perf_event *event,
> >   	if (unlikely(!is_sampling_event(event)))
> >   		return 0;
> > -	ret = __perf_event_account_interrupt(event, throttle);
> > +	/*
> > +	 * High Freq samples are injected inside the larger period:
> > +	 *
> > +	 *   |------------|-|------------|-|
> > +	 *   P0          HF P1          HF
> > +	 *
> > +	 * By ignoring the HF samples, we measure the actual period.
> > +	 */
> > +	if (!(hwc->state & PERF_HES_HF_SAMPLE))
> > +		ret = __perf_event_account_interrupt(event, throttle);
> 
> The high-frequency samples should still contribute to interrupt
> accounting/throttling, right? We'd just need to put guards around the
> adaptive period stuff so that HF samples don't contribute to the frequency
> training.

Yeah, I suppose it should. This means breaking up that function but that
isn't hard.
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Tue, Mar 11, 2025 at 12:31:29PM +0100, Peter Zijlstra wrote:
> +	sample_period = hwc->sample_period_base;
> +	if (hwc->state & PERF_HES_HF_RAND) {
> +		u64 rand = 1 << event->attr.hf_sample_rand;
> +		sample_period -= rand / 2;
> +		sample_period += prandom_u32_state(&perf_rand) & (rand - 1);
> +	}
> +	if (hwc->state & PERF_HES_HF_ON) {
> +		u64 hf_sample_period = event->attr.hf_sample_period;
> +
> +		if (sample_period < hf_sample_period) {
> +			hwc->state &= ~PERF_HES_HF_ON;
> +			goto set_period;
> +		}
> +
> +		if (!(hwc->state & PERF_HES_HF_SAMPLE)) {
> +			hwc->sample_period -= hf_sample_period;
> +			hwc->state |= PERF_HES_HF_SAMPLE;
> +		} else {
> +			hwc->sample_period = hf_sample_period;
> +			hwc->state &= ~PERF_HES_HF_SAMPLE;

and obviously this should be the local sample_period modified above, not
the hwc one.

> +		}
> +	}
> +set_period:
> +	hwc->sample_period = sample_period;
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Tue, Mar 11, 2025 at 12:31:29PM +0100, Peter Zijlstra wrote:
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index b87a5ac42ce2..e5a93edf3b5f 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -8,6 +8,7 @@
>   *  Copyright  ©  2009 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
>   */
>  
> +#include "linux/random.h"
>  #include <linux/fs.h>
>  #include <linux/mm.h>
>  #include <linux/cpu.h>

Argh, this is neovim trying to be 'helpful'. If anybody reading this
knows how to make it stop adding headers, please let me know, its
driving me nuts.
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Tue, Mar 11, 2025 at 12:35:30PM +0100, Peter Zijlstra wrote:
> On Tue, Mar 11, 2025 at 12:31:29PM +0100, Peter Zijlstra wrote:
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index b87a5ac42ce2..e5a93edf3b5f 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -8,6 +8,7 @@
> >   *  Copyright  ©  2009 Paul Mackerras, IBM Corp. <paulus@au1.ibm.com>
> >   */
> >  
> > +#include "linux/random.h"
> >  #include <linux/fs.h>
> >  #include <linux/mm.h>
> >  #include <linux/cpu.h>
> 
> Argh, this is neovim trying to be 'helpful'. If anybody reading this
> knows how to make it stop adding headers, please let me know, its
> driving me nuts.

Adding this cmd thing to the lspconfig like so:

require 'lspconfig'.clangd.setup() {
	cmd = {
		"clangd",
		"--header-insertion=never",
	},
}

seems to do the trick.
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Peter Zijlstra 11 months ago
On Fri, Mar 07, 2025 at 08:22:45PM +0000, mark.barnett@arm.com wrote:
> @@ -9922,7 +9923,10 @@ static int __perf_event_overflow(struct perf_event *event,
>  	if (event->attr.alt_sample_period) {
>  		bool using_alt = hwc->using_alt_sample_period;
>  		u64 sample_period = (using_alt ? event->attr.sample_period
> -					       : event->attr.alt_sample_period);
> +					       : event->attr.alt_sample_period)
> +				  + (event->attr.jitter_alt_period
> +					? get_random_u32_below(2 << event->attr.jitter_alt_period)
> +					: 0);

So, ... this here is NMI context, right? Have you looked at the guts of
get_random_u32_below() ?

I would strongly suggest you go do so.

>  
>  		hwc->sample_period = sample_period;
>  		hwc->using_alt_sample_period = !using_alt;
Re: [PATCH v3 3/5] perf: Allow adding fixed random jitter to the alternate sampling period
Posted by Mark Barnett 11 months ago
On 3/10/25 12:47, Peter Zijlstra wrote:
> On Fri, Mar 07, 2025 at 08:22:45PM +0000, mark.barnett@arm.com wrote:
>> @@ -9922,7 +9923,10 @@ static int __perf_event_overflow(struct perf_event *event,
>>   	if (event->attr.alt_sample_period) {
>>   		bool using_alt = hwc->using_alt_sample_period;
>>   		u64 sample_period = (using_alt ? event->attr.sample_period
>> -					       : event->attr.alt_sample_period);
>> +					       : event->attr.alt_sample_period)
>> +				  + (event->attr.jitter_alt_period
>> +					? get_random_u32_below(2 << event->attr.jitter_alt_period)
>> +					: 0);
> 
> So, ... this here is NMI context, right? Have you looked at the guts of
> get_random_u32_below() ?
> 
> I would strongly suggest you go do so.
> 

Good catch. I think a pseudo-random generator would be fine here and it 
looks like the implementation of prandom is safe to use in an interrupt 
context. I can change to use that.

>>   
>>   		hwc->sample_period = sample_period;
>>   		hwc->using_alt_sample_period = !using_alt;