All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Stephane Eranian <eranian@google.com>,
	Jiri Olsa <jolsa@redhat.com>,
	stable@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>
Subject: [patch 1/5] perf: Fixup hrtimer forward wreckage
Date: Mon, 13 Apr 2015 21:02:18 -0000	[thread overview]
Message-ID: <20150413210035.020255651@linutronix.de> (raw)
In-Reply-To: 20150413210009.682000343@linutronix.de

[-- Attachment #1: perf-remove-hrtimer-forward-wreckage.patch --]
[-- Type: text/plain, Size: 3209 bytes --]

perf_event_mux_interval_ms_store() tries to apply an updated
hrtimer_interval to all possible cpus in a completely unsafe way. The
changelog of the offending commit says:

 "In the 5th version, we handle the reprogramming of the hrtimer using
  hrtimer_forward_now(). That way, we sync up to new timer value
  quickly (suggested by Jiri Olsa)."

The hrtimer reprogramming is completely unrelated to
hrtimer_forward_now(). hrtimer_forward_now() merily forwards the
expiry time of the hrtimer past now and that's where things get really
bad in this code:

The forward function is called on enqueued timers. That means the
update of the expiry time corrupts the ordering of the hrtimer rbtree.

The proper way to update hrtimers on remote cpus is to use a smp
function call on all online cpus and perform the update locally by
canceling the timer, forwarding and restarting it.

Fixes: 62b856397927 "perf: Add sysfs entry to adjust multiplexing interval per PMU"
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Stephane Eranian <eranian@google.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: stable@vger.kernel.org
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 kernel/events/core.c |   36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Index: linux/kernel/events/core.c
===================================================================
--- linux.orig/kernel/events/core.c
+++ linux/kernel/events/core.c
@@ -6827,13 +6827,27 @@ perf_event_mux_interval_ms_show(struct d
 	return snprintf(page, PAGE_SIZE-1, "%d\n", pmu->hrtimer_interval_ms);
 }
 
+static void perf_event_mux_update_interval(void *info)
+{
+	struct pmu *pmu = info;
+	struct perf_cpu_context *cpuctx = this_cpu_ptr(pmu->pmu_cpu_context);
+	int was_armed = hrtimer_cancel(&cpuctx->hrtimer);
+
+	/* Update the interval and restart the timer with the new interval */
+	cpuctx->hrtimer_interval = ms_to_ktime(pmu->hrtimer_interval_ms);
+	if (was_armed) {
+		hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
+		hrtimer_start_expires(&cpuctx->hrtimer, HRTIMER_MODE_ABS);
+	}
+}
+
 static ssize_t
 perf_event_mux_interval_ms_store(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
 	struct pmu *pmu = dev_get_drvdata(dev);
-	int timer, cpu, ret;
+	int timer, ret;
 
 	ret = kstrtoint(buf, 0, &timer);
 	if (ret)
@@ -6842,22 +6856,12 @@ perf_event_mux_interval_ms_store(struct
 	if (timer < 1)
 		return -EINVAL;
 
-	/* same value, noting to do */
-	if (timer == pmu->hrtimer_interval_ms)
-		return count;
-
-	pmu->hrtimer_interval_ms = timer;
-
-	/* update all cpuctx for this PMU */
-	for_each_possible_cpu(cpu) {
-		struct perf_cpu_context *cpuctx;
-		cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-		cpuctx->hrtimer_interval = ns_to_ktime(NSEC_PER_MSEC * timer);
-
-		if (hrtimer_active(&cpuctx->hrtimer))
-			hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
+	if (timer != pmu->hrtimer_interval_ms) {
+		get_online_cpus();
+		pmu->hrtimer_interval_ms = timer;
+		on_each_cpu(perf_event_mux_update_interval, pmu, 1);
+		put_online_cpus();
 	}
-
 	return count;
 }
 static DEVICE_ATTR_RW(perf_event_mux_interval_ms);



  reply	other threads:[~2015-04-13 21:02 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-13 21:02 [patch 0/5] hrtimer: Cleanup usage trainwrecks treewide Thomas Gleixner
2015-04-13 21:02 ` Thomas Gleixner [this message]
2015-04-14 22:10   ` [patch 1/5] perf: Fixup hrtimer forward wreckage Stephane Eranian
2015-04-13 21:02 ` [patch 2/5] s390: crypto: Protect poll timeout update Thomas Gleixner
2015-04-17 13:27   ` Ingo Tuchscherer
2015-04-13 21:02 ` [patch 3/5] hrtimer: Document hrtimer_forward[_now]() proper Thomas Gleixner
2015-04-22 19:04   ` [tip:timers/core] " tip-bot for Thomas Gleixner
2015-04-13 21:02 ` [patch 4/5] net: hip04: Make tx coalesce timer actually work Thomas Gleixner
2015-04-13 21:24   ` Arnd Bergmann
2015-04-13 21:42     ` Thomas Gleixner
2015-04-13 22:03       ` Arnd Bergmann
2015-04-13 22:08         ` Thomas Gleixner
2015-04-14  7:53           ` Ding Tianhong
2015-04-14 18:15           ` David Miller
2015-04-14 19:42             ` [patch v2] " Thomas Gleixner
2015-04-15  2:24               ` Ding Tianhong
2015-04-15 10:20               ` Arnd Bergmann
2015-04-15 21:22               ` David Miller
2015-04-13 21:02 ` [patch 5/5] staging: ozwpan: Fix hrtimer wreckage Thomas Gleixner
2015-04-13 21:25   ` Greg Kroah-Hartman
2015-07-20  9:40   ` [tip:timers/core] " tip-bot for Thomas Gleixner

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=20150413210035.020255651@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=acme@kernel.org \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    /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.