hrtimer group events

Don't open code the event check for excluding the idle thread. Instead include the check in perf_exclude_event().

Signed-off-by Matt Fleming <[email protected]>

---
 kernel/perf_event.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

kernel/perf_event.c +0 -0

index 0d38f27..16b0476 100644

Quoted 13 lines · Matt Fleming

Don't open code the event check for excluding the idle thread. Instead include the check in perf_exclude_event().

Signed-off-by Matt Fleming <[email protected]>

---
 kernel/perf_event.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

kernel/perf_event.c +0 -0

index 0d38f27..16b0476 100644

kernel/perf_event.c static int perf_exclude_event(struct perf_event *event, · line 4315

Quoted 6 lines

kernel/perf_event.c +3 -0

@@ -4310,6 +4310,9 @@ static int perf_exclude_event(struct perf_event *event,
 
 		if (event->attr.exclude_kernel && !user_mode(regs))
 			return 1;
+
+		if (event->attr.exclude_idle && current->pid == 0)
+			return 1;

Right.

But one of the problems people have reported is that they can miss interrupts samples if they happen in idle. Hence we have decided that exclude_idle should exclude events that happen in idle process context but not in interrupts interrupting idle.

So adding an in_interrupt() check would perhaps be better.

I plan to do this exclusion using the per context exclusion, which is a patchset I have in queue. But until then, having this patch is better.

kernel/perf_event.c static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer) · line 4519

Quoted 12 lines

kernel/perf_event.c +2 -3

 	}
 
 	return 0;

@@ -4512,9 +4515,8 @@ static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer)
 	regs = get_irq_regs();
 
 	if (regs && !perf_exclude_event(event, regs)) {
-		if (!(event->attr.exclude_idle && current->pid == 0))
-			if (perf_event_overflow(event, 0, &data, regs))
-				ret = HRTIMER_NORESTART;
+		if (perf_event_overflow(event, 0, &data, regs))
+			ret = HRTIMER_NORESTART;

But yeah if we add an in_interrupt() check in perf_exclude_event(), it won't work here. This one needs to check if irqs are nesting :)

Bah, checking we interrupted softirqs is probably enough. I guess we don't care about nesting hardirqs.

Quoted 55 lines · Frederic Weisbecker

Quoted 13 lines · Matt Fleming

Don't open code the event check for excluding the idle thread. Instead include the check in perf_exclude_event().

Signed-off-by Matt Fleming <[email protected]>

---
 kernel/perf_event.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

kernel/perf_event.c +0 -0

index 0d38f27..16b0476 100644

kernel/perf_event.c static int perf_exclude_event(struct perf_event *event, · line 4315

Quoted 6 lines

kernel/perf_event.c +3 -0

@@ -4310,6 +4310,9 @@ static int perf_exclude_event(struct perf_event *event,
 
 		if (event->attr.exclude_kernel && !user_mode(regs))
 			return 1;
+
+		if (event->attr.exclude_idle && current->pid == 0)
+			return 1;

Right.

But one of the problems people have reported is that they can miss interrupts samples if they happen in idle. Hence we have decided that exclude_idle should exclude events that happen in idle process context but not in interrupts interrupting idle.

So adding an in_interrupt() check would perhaps be better.

I plan to do this exclusion using the per context exclusion, which is a patchset I have in queue. But until then, having this patch is better.

kernel/perf_event.c static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer) · line 4519

Quoted 12 lines

kernel/perf_event.c +2 -3

 	}
 
 	return 0;

@@ -4512,9 +4515,8 @@ static enum hrtimer_restart perf_swevent_hrtimer(struct hrtimer *hrtimer)
 	regs = get_irq_regs();
 
 	if (regs && !perf_exclude_event(event, regs)) {
-		if (!(event->attr.exclude_idle && current->pid == 0))
-			if (perf_event_overflow(event, 0, &data, regs))
-				ret = HRTIMER_NORESTART;
+		if (perf_event_overflow(event, 0, &data, regs))
+			ret = HRTIMER_NORESTART;

But yeah if we add an in_interrupt() check in perf_exclude_event(), it won't work here. This one needs to check if irqs are nesting :)

Quoted 2 lines

Bah, checking we interrupted softirqs is probably enough. I guess we don't care about nesting hardirqs.

This patch isn't really worth it on its own, I only grouped the idle check into perf_exclude_event() because patch 3/5 introduced a new caller. As you've said, the semantics at the various callsites are quite complex. It's probably best to wait for your patchset :)

Change the semantics of the PERF_FORMAT_GROUP records and make them delta values, i.e. the difference in value between consecutive reads of the hardware counters. These delta values will be used in a subsequent patch to calculate weighted values (by multiplying the deltas with the time difference between reads of the counters). Calculating these deltas is easiest to do in the kernel (as opposed to the userland tools) because we can reset the counter whenever we read from it.

Note that even though this patch changes the semantics of PERF_FORMAT_GROUP records there were no users of them anyway (the tools didn't even understand them).

Signed-off-by Matt Fleming <[email protected]>

---
 kernel/perf_event.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

kernel/perf_event.c +10 -0

index 16b0476..2cda375 100644
@@ -3450,7 +3450,14 @@ static void perf_output_read_group(struct perf_output_handle *handle,
 	if (leader != event)
 		leader->pmu->read(leader);
 
+	/*
+	 * Reset the counter value so that ->count contains a delta from
+	 * the previous value.
+	 */
 	values[n++] = perf_event_count(leader);
+	local64_set(&leader->count, 0);
+	atomic64_set(&leader->child_count, 0);
+
 	if (read_format & PERF_FORMAT_ID)
 		values[n++] = primary_event_id(leader);
 

@@ -3463,6 +3470,9 @@ static void perf_output_read_group(struct perf_output_handle *handle,
 			sub->pmu->read(sub);
 
 		values[n++] = perf_event_count(sub);
+		local64_set(&sub->count, 0);
+		atomic64_set(&sub->child_count, 0);
+
 		if (read_format & PERF_FORMAT_ID)
 			values[n++] = primary_event_id(sub);
 

Quoted 11 lines · Matt Fleming

Change the semantics of the PERF_FORMAT_GROUP records and make them delta values, i.e. the difference in value between consecutive reads of the hardware counters. These delta values will be used in a subsequent patch to calculate weighted values (by multiplying the deltas with the time difference between reads of the counters). Calculating these deltas is easiest to do in the kernel (as opposed to the userland tools) because we can reset the counter whenever we read from it.

Note that even though this patch changes the semantics of PERF_FORMAT_GROUP records there were no users of them anyway (the tools didn't even understand them).

libpfmon might be using it,.. I know some people occasionally reported bugs for this so there are users.

Also, there's no real synchonization between read,read and read,sample so its racy to do this, simply generate deltas in post-processing.

Currently, it's impossible to periodically sample hardware counters that lack performance monitoring interrupt (PMI) support. In order to sample these counters we can create an event group which is backed by a hrtimer, thereby simulating a PMI.

When the hrtimer goes off we sample the values in the hardware counters. Because we obviously can't rely on the hrtimer going off at exactly the sample period (say, every 1000 cache misses) the values need to be weighted by the variable period since the last hrtimer went off. This is so that we can compensate for the variability in hrtimer period.

If perf record tries to create a sampling counter and the hardware doesn't support it then we'll fall back to creating an event group with a hrtimer.

Quoted 12 lines · Matt Fleming

Currently, it's impossible to periodically sample hardware counters that lack performance monitoring interrupt (PMI) support. In order to sample these counters we can create an event group which is backed by a hrtimer, thereby simulating a PMI.

When the hrtimer goes off we sample the values in the hardware counters. Because we obviously can't rely on the hrtimer going off at exactly the sample period (say, every 1000 cache misses) the values need to be weighted by the variable period since the last hrtimer went off. This is so that we can compensate for the variability in hrtimer period.

Quoted 3 lines

If perf record tries to create a sampling counter and the hardware doesn't support it then we'll fall back to creating an event group with a hrtimer.

Why is this changing kernel code?

You can create those groups in userspace..

I'm lost. Is it possible to do this patch entirely in userspace? How do we periodically sample the counters if it's not being done in the kernel?

Quoted 3 lines · Matt Fleming

I'm lost. Is it possible to do this patch entirely in userspace? How do we periodically sample the counters if it's not being done in the kernel?

perf_event_attr leader = {
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_TASK_CLOCK,
.sample_period = xxx,
.sample_type = PERF_SAMPLE_READ|...,
.read_format = PERF_FORMAT_GROUP,
};

perf_event_attr counter = {
.type = PERF_TYPE_HARDWARE,
.config = PERF_COUNT_HW_CPU_CYCLES,
};

leader_fd = sys_perf_event_open(&leader, pid, cpu, 0, 0); counter_fd = sys_perf_event_open(&counter, pid, cpu, leader_fd, 0);

Which gives you a group of 2 events, one software timer that samples, one hardware counter that only counts.