All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@kernel.org>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: lkml <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Andi Kleen <andi@firstfloor.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Vince Weaver <vince@deater.net>
Subject: [PATCH 4/4] perf/x86/intel: Throttle PEBS events only from pmi
Date: Wed, 28 Dec 2016 14:31:06 +0100	[thread overview]
Message-ID: <1482931866-6018-5-git-send-email-jolsa@kernel.org> (raw)
In-Reply-To: <1482931866-6018-1-git-send-email-jolsa@kernel.org>

This patch fixes following WARNING:

  WARNING: CPU: 15 PID: 15768 at arch/x86/events/core.c:1256 x86_pmu_start+0x1b3/0x1c0
  ...
  Call Trace:
   <IRQ>
   dump_stack+0x86/0xc3
   __warn+0xcb/0xf0
   warn_slowpath_null+0x1d/0x20
   x86_pmu_start+0x1b3/0x1c0
   perf_event_task_tick+0x342/0x3f0
   scheduler_tick+0x75/0xd0
   update_process_times+0x47/0x60
   tick_sched_handle.isra.19+0x25/0x60
   tick_sched_timer+0x3d/0x70
   __hrtimer_run_queues+0xfb/0x510
   hrtimer_interrupt+0x9d/0x1a0
   local_apic_timer_interrupt+0x38/0x60
   smp_trace_apic_timer_interrupt+0x56/0x25a
   trace_apic_timer_interrupt+0x9d/0xb0
   ...

which happens AFAICS under following conditions:
(we have PEBS events configured)

  - x86_pmu_enable reconfigures counters and calls:
       - x86_pmu_stop on PEBS event
       - x86_pmu_stop drains the PEBS buffer, crosses
         the throttle limit and sets:
           event->hw.interrupts = MAX_INTERRUPTS
       - following x86_pmu_start call starts the event
  - perf_event_task_tick is triggered
    - perf_adjust_freq_unthr_context sees event with
      MAX_INTERRUPTS set and calls x86_pmu_start on already
      started event, which triggers the warning

My first attempt to fix this was to unthrottle the event
before starting it in x86_pmu_enable. But I think that
omitting the throttling completely when we are not in the
PMI is better.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 arch/x86/events/intel/core.c |  2 +-
 arch/x86/events/intel/ds.c   | 17 +++++++++--------
 arch/x86/events/perf_event.h |  2 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 86138267b68a..16332867d435 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2137,7 +2137,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
 	 */
 	if (__test_and_clear_bit(62, (unsigned long *)&status)) {
 		handled++;
-		x86_pmu.drain_pebs(regs);
+		x86_pmu.drain_pebs(regs, 1);
 		status &= x86_pmu.intel_ctrl | GLOBAL_STATUS_TRACE_TOPAPMI;
 	}
 
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 9dfeeeca0ea8..75ba3cb06012 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -603,7 +603,7 @@ static inline void intel_pmu_drain_pebs_buffer(void)
 {
 	struct pt_regs regs;
 
-	x86_pmu.drain_pebs(&regs);
+	x86_pmu.drain_pebs(&regs, 0);
 }
 
 void intel_pmu_pebs_sched_task(struct perf_event_context *ctx, bool sched_in)
@@ -1233,7 +1233,7 @@ get_next_pebs_record_by_bit(void *base, void *top, int bit)
 static void __intel_pmu_pebs_event(struct perf_event *event,
 				   struct pt_regs *iregs,
 				   void *base, void *top,
-				   int bit, int count)
+				   int bit, int count, int pmi)
 {
 	struct perf_sample_data data;
 	struct pt_regs regs;
@@ -1257,14 +1257,14 @@ static void __intel_pmu_pebs_event(struct perf_event *event,
 	 * 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)) {
+	if (perf_event_overflow_throttle(event, pmi, &data, &regs)) {
 		x86_pmu_stop(event, 0);
 		return;
 	}
 
 }
 
-static void intel_pmu_drain_pebs_core(struct pt_regs *iregs)
+static void intel_pmu_drain_pebs_core(struct pt_regs *iregs, int pmi)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 	struct debug_store *ds = cpuc->ds;
@@ -1295,10 +1295,10 @@ static void intel_pmu_drain_pebs_core(struct pt_regs *iregs)
 	if (n <= 0)
 		return;
 
-	__intel_pmu_pebs_event(event, iregs, at, top, 0, n);
+	__intel_pmu_pebs_event(event, iregs, at, top, 0, n, pmi);
 }
 
-static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
+static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs, int pmi)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 	struct debug_store *ds = cpuc->ds;
@@ -1392,13 +1392,14 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
 		if (error[bit]) {
 			perf_log_lost_samples(event, error[bit]);
 
-			if (perf_event_account_interrupt(event))
+			if (pmi && perf_event_account_interrupt(event))
 				x86_pmu_stop(event, 0);
 		}
 
 		if (counts[bit]) {
 			__intel_pmu_pebs_event(event, iregs, base,
-					       top, bit, counts[bit]);
+					       top, bit, counts[bit],
+					       pmi);
 		}
 	}
 }
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index bcbb1d2ae10b..cdcfdd06b2d2 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -590,7 +590,7 @@ struct x86_pmu {
 			pebs_prec_dist	:1;
 	int		pebs_record_size;
 	int		pebs_buffer_size;
-	void		(*drain_pebs)(struct pt_regs *regs);
+	void		(*drain_pebs)(struct pt_regs *regs, int pmi);
 	struct event_constraint *pebs_constraints;
 	void		(*pebs_aliases)(struct perf_event *event);
 	int 		max_pebs_events;
-- 
2.7.4

  parent reply	other threads:[~2016-12-28 13:31 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-28 13:31 [PATCH 0/4] perf: Fuzzer fixes Jiri Olsa
2016-12-28 13:31 ` [PATCH 1/4] perf/x86/intel: Account interrupts for PEBS errors Jiri Olsa
2017-01-14 12:29   ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2016-12-28 13:31 ` [PATCH 2/4] perf/x86: Fix period for non sampling events Jiri Olsa
2017-01-03  9:40   ` Peter Zijlstra
2017-01-03 14:24     ` [PATCH] perf/x86: Reject non sampling events with precise_ip Jiri Olsa
2017-01-03 22:06       ` Vince Weaver
2017-01-14 12:29       ` [tip:perf/urgent] " tip-bot for Jiri Olsa
2017-01-03 15:09   ` [PATCH 2/4] perf/x86: Fix period for non sampling events Peter Zijlstra
2017-01-03 15:26     ` Jiri Olsa
2016-12-28 13:31 ` [PATCH 3/4] perf: Add perf_event_overflow_throttle function Jiri Olsa
2016-12-28 13:31 ` Jiri Olsa [this message]
2017-01-03 13:45   ` [PATCH 4/4] perf/x86/intel: Throttle PEBS events only from pmi Peter Zijlstra
2017-01-24 16:41   ` Peter Zijlstra
2017-01-25 13:02     ` Jiri Olsa
2017-01-25 13:02     ` Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1482931866-6018-5-git-send-email-jolsa@kernel.org \
    --to=jolsa@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=vince@deater.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.