lkml.org 
[lkml]   [2017]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/4] perf/x86/intel: Throttle PEBS events only from pmi
On Tue, Jan 24, 2017 at 05:41:22PM +0100, Peter Zijlstra wrote:

SNIP

>
> So I don't particularly like these patches... they make a wee bit of a
> mess.
>
> Under the assumption that draining a single event is on the same order
> of cost as a regular PMI, then accounting a drain of multiple events as
> an equal amount of interrupts makes sense.

I understood it like we're trying more to protect the interrupt
context abuse rather than account the samples here.. but yes,
we compare (hwc->interrupts >= max_samples_per_tick) for throttling
so I guess we should not cross max_samples_per_tick in any case

>
> We should not disregard this work. Now it looks like both (BTS & PEBS)
> drain methods only count a single interrupt, that's something we maybe
> ought to fix too.

right, we account just single hwc->interrupts++, and BTS drain does
not have the throttle logic

I tried to put the perf_event_overflow call into both PEBS and BTS
drains (below, untested). I think it's going to be slower especially
for BTS code, which bypassed single event allocations and allocated
one buffer for all samples at once.

jirka


---
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index be202390bbd3..703bd82c69b1 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -520,10 +520,7 @@ int intel_pmu_drain_bts_buffer(void)
};
struct perf_event *event = cpuc->events[INTEL_PMC_IDX_FIXED_BTS];
struct bts_record *at, *base, *top;
- struct perf_output_handle handle;
- struct perf_event_header header;
struct perf_sample_data data;
- unsigned long skip = 0;
struct pt_regs regs;

if (!event)
@@ -562,40 +559,18 @@ int intel_pmu_drain_bts_buffer(void)
*/
if (event->attr.exclude_kernel &&
(kernel_ip(at->from) || kernel_ip(at->to)))
- skip++;
- }
-
- /*
- * Prepare a generic sample, i.e. fill in the invariant fields.
- * We will overwrite the from and to address before we output
- * the sample.
- */
- rcu_read_lock();
- perf_prepare_sample(&header, &data, event, &regs);
-
- if (perf_output_begin(&handle, event, header.size *
- (top - base - skip)))
- goto unlock;
-
- for (at = base; at < top; at++) {
- /* Filter out any records that contain kernel addresses. */
- if (event->attr.exclude_kernel &&
- (kernel_ip(at->from) || kernel_ip(at->to)))
continue;

data.ip = at->from;
data.addr = at->to;

- perf_output_sample(&handle, &header, &data, event);
- }
+ if (perf_event_overflow(event, &data, &regs)) {
+ x86_pmu_stop(event, 0);
+ break;
+ }

- perf_output_end(&handle);
+ }

- /* There's new data available. */
- event->hw.interrupts++;
- event->pending_kill = POLL_IN;
-unlock:
- rcu_read_unlock();
return 1;
}

@@ -1243,25 +1218,18 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
!(event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD))
return;

- while (count > 1) {
+ while (count > 0) {
setup_pebs_sample_data(event, iregs, at, &data, &regs);
- perf_event_output(event, &data, &regs);
+
+ if (perf_event_overflow(event, &data, &regs)) {
+ x86_pmu_stop(event, 0);
+ break;
+ }
+
at += x86_pmu.pebs_record_size;
at = get_next_pebs_record_by_bit(at, top, bit);
count--;
}
-
- setup_pebs_sample_data(event, iregs, at, &data, &regs);
-
- /*
- * All but the last records are processed.
- * The last one is left to be able to call the overflow handler.
- */
- if (perf_event_overflow(event, &data, &regs)) {
- x86_pmu_stop(event, 0);
- return;
- }
-
}

static void intel_pmu_drain_pebs_core(struct pt_regs *iregs)
\
 
 \ /
  Last update: 2017-01-25 14:02    [W:0.116 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site