Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1342271 > unrolled thread

[PATCH 00/12] perf: more fixes

Started byPeter Zijlstra <peterz@infradead.org>
First post2016-02-24 19:00 +0100
Last post2016-02-25 09:10 +0100
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] perf: more fixes Peter Zijlstra <peterz@infradead.org> - 2016-02-24 19:00 +0100
    [PATCH 09/12] perf: Fix scaling vs enable_on_exec Peter Zijlstra <peterz@infradead.org> - 2016-02-24 19:00 +0100
      [tip:perf/urgent] perf: Fix scaling vs. perf_event_enable_on_exec() tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-02-25 09:10 +0100
    [PATCH 02/12] perf: Do not double free Peter Zijlstra <peterz@infradead.org> - 2016-02-24 19:00 +0100
      [tip:perf/urgent] perf: Do not double free tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-02-25 09:10 +0100
    [PATCH 07/12] perf: Cure event->pending_disable race Peter Zijlstra <peterz@infradead.org> - 2016-02-24 19:00 +0100
      [tip:perf/urgent] perf: Cure event->pending_disable race tip-bot for Peter Zijlstra <tipbot@zytor.com> - 2016-02-25 09:10 +0100

#1342271 — [PATCH 00/12] perf: more fixes

FromPeter Zijlstra <peterz@infradead.org>
Date2016-02-24 19:00 +0100
Subject[PATCH 00/12] perf: more fixes
Message-ID<r5LTc-7ZZ-11@gated-at.bofh.it>
About half of these patches are syz-kaller inspired, the rest is reported
fallout from the last round of fixes.

With these patches syz-kaller can still trigger some fail; most notably some
NMI watchdog triggers and a very sporadic unthrottle bug (much like last time).
But stability is otherwise greatly improved.

Please all, have a good hard look at these patches.

Thanks to Alexander for really spending some time with them.

[toc] | [next] | [standalone]

#1342272 — [PATCH 09/12] perf: Fix scaling vs enable_on_exec

FromPeter Zijlstra <peterz@infradead.org>
Date2016-02-24 19:00 +0100
Subject[PATCH 09/12] perf: Fix scaling vs enable_on_exec
Message-ID<r5LTd-7ZZ-33@gated-at.bofh.it>
In reply to#1342271
The recent commit 3e349507d12d ("perf: Fix perf_enable_on_exec() event
scheduling") caused this by moving task_ctx_sched_out() from before
__perf_event_mask_enable() to after it.

The overlooked concequence of that change is that task_ctx_sched_out()
would update the ctx time fields, and now __perf_event_mask_enable()
uses stale time.

In order to fix this, explicitly stop our context's time before
enabling the event(s).

Fixes: 3e349507d12d ("perf: Fix perf_enable_on_exec() event scheduling")
Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3143,6 +3143,7 @@ static void perf_event_enable_on_exec(in
 
 	cpuctx = __get_cpu_context(ctx);
 	perf_ctx_lock(cpuctx, ctx);
+	ctx_sched_out(ctx, cpuctx, EVENT_TIME);
 	list_for_each_entry(event, &ctx->event_list, event_entry)
 		enabled |= event_enable_on_exec(event, ctx);

[toc] | [prev] | [next] | [standalone]

#1342935 — [tip:perf/urgent] perf: Fix scaling vs. perf_event_enable_on_exec()

Fromtip-bot for Peter Zijlstra <tipbot@zytor.com>
Date2016-02-25 09:10 +0100
Subject[tip:perf/urgent] perf: Fix scaling vs. perf_event_enable_on_exec()
Message-ID<r5Z9L-U0-9@gated-at.bofh.it>
In reply to#1342272
Commit-ID:  7fce250915efca0f8f51dddee3ae89bf30d86ca5
Gitweb:     http://git.kernel.org/tip/7fce250915efca0f8f51dddee3ae89bf30d86ca5
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 24 Feb 2016 18:45:48 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 25 Feb 2016 08:43:34 +0100

perf: Fix scaling vs. perf_event_enable_on_exec()

The recent commit 3e349507d12d ("perf: Fix perf_enable_on_exec() event
scheduling") caused this by moving task_ctx_sched_out() from before
__perf_event_mask_enable() to after it.

The overlooked consequence of that change is that task_ctx_sched_out()
would update the ctx time fields, and now __perf_event_mask_enable()
uses stale time.

In order to fix this, explicitly stop our context's time before
enabling the event(s).

Reported-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dvyukov@google.com
Cc: eranian@google.com
Cc: panand@redhat.com
Cc: sasha.levin@oracle.com
Cc: vince@deater.net
Fixes: 3e349507d12d ("perf: Fix perf_enable_on_exec() event scheduling")
Link: http://lkml.kernel.org/r/20160224174948.159242158@infradead.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 75bde93..d003088 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3143,6 +3143,7 @@ static void perf_event_enable_on_exec(int ctxn)
 
 	cpuctx = __get_cpu_context(ctx);
 	perf_ctx_lock(cpuctx, ctx);
+	ctx_sched_out(ctx, cpuctx, EVENT_TIME);
 	list_for_each_entry(event, &ctx->event_list, event_entry)
 		enabled |= event_enable_on_exec(event, ctx);

[toc] | [prev] | [next] | [standalone]

#1342273 — [PATCH 02/12] perf: Do not double free

FromPeter Zijlstra <peterz@infradead.org>
Date2016-02-24 19:00 +0100
Subject[PATCH 02/12] perf: Do not double free
Message-ID<r5LTd-7ZZ-35@gated-at.bofh.it>
In reply to#1342271
In case of: err_file: fput(event_file), we'll end up calling
perf_release() which in turn will free the event.

Do not then free the event _again_.

Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8520,7 +8520,12 @@ SYSCALL_DEFINE5(perf_event_open,
 	perf_unpin_context(ctx);
 	put_ctx(ctx);
 err_alloc:
-	free_event(event);
+	/*
+	 * If event_file is set, the fput() above will have called ->release()
+	 * and that will take care of freeing the event.
+	 */
+	if (!event_file)
+		free_event(event);
 err_cpus:
 	put_online_cpus();
 err_task:

[toc] | [prev] | [next] | [standalone]

#1342936 — [tip:perf/urgent] perf: Do not double free

Fromtip-bot for Peter Zijlstra <tipbot@zytor.com>
Date2016-02-25 09:10 +0100
Subject[tip:perf/urgent] perf: Do not double free
Message-ID<r5Z9L-U0-11@gated-at.bofh.it>
In reply to#1342273
Commit-ID:  130056275ade730e7a79c110212c8815202773ee
Gitweb:     http://git.kernel.org/tip/130056275ade730e7a79c110212c8815202773ee
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 24 Feb 2016 18:45:41 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 25 Feb 2016 08:42:32 +0100

perf: Do not double free

In case of: err_file: fput(event_file), we'll end up calling
perf_release() which in turn will free the event.

Do not then free the event _again_.

Tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dvyukov@google.com
Cc: eranian@google.com
Cc: oleg@redhat.com
Cc: panand@redhat.com
Cc: sasha.levin@oracle.com
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/20160224174947.697350349@infradead.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index d7b0316..211a5ce 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8520,7 +8520,12 @@ err_context:
 	perf_unpin_context(ctx);
 	put_ctx(ctx);
 err_alloc:
-	free_event(event);
+	/*
+	 * If event_file is set, the fput() above will have called ->release()
+	 * and that will take care of freeing the event.
+	 */
+	if (!event_file)
+		free_event(event);
 err_cpus:
 	put_online_cpus();
 err_task:

[toc] | [prev] | [next] | [standalone]

#1342274 — [PATCH 07/12] perf: Cure event->pending_disable race

FromPeter Zijlstra <peterz@infradead.org>
Date2016-02-24 19:00 +0100
Subject[PATCH 07/12] perf: Cure event->pending_disable race
Message-ID<r5LTd-7ZZ-37@gated-at.bofh.it>
In reply to#1342271
Because event_sched_out() checks event->pending_disable _before_
actually disabling the event, it can happen that the event fires after
it checks but before it gets disabled.

This would leave event->pending_disable set and the queued irq_work
will try and process it.

However, if the event trigger was during schedule(), the event might
have been de-scheduled by the time the irq_work runs, and
perf_event_disable_local() will fail.

Fix this by checking event->pending_disable _after_ we call
event->pmu->del(). This depends on the latter being a compiler
barrier, such that the compiler does not lift the load and re-creates
the problem.

Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/events/core.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1696,14 +1696,14 @@ event_sched_out(struct perf_event *event
 
 	perf_pmu_disable(event->pmu);
 
+	event->tstamp_stopped = tstamp;
+	event->pmu->del(event, 0);
+	event->oncpu = -1;
 	event->state = PERF_EVENT_STATE_INACTIVE;
 	if (event->pending_disable) {
 		event->pending_disable = 0;
 		event->state = PERF_EVENT_STATE_OFF;
 	}
-	event->tstamp_stopped = tstamp;
-	event->pmu->del(event, 0);
-	event->oncpu = -1;
 
 	if (!is_software_event(event))
 		cpuctx->active_oncpu--;

[toc] | [prev] | [next] | [standalone]

#1342944 — [tip:perf/urgent] perf: Cure event->pending_disable race

Fromtip-bot for Peter Zijlstra <tipbot@zytor.com>
Date2016-02-25 09:10 +0100
Subject[tip:perf/urgent] perf: Cure event->pending_disable race
Message-ID<r5Z9M-U0-33@gated-at.bofh.it>
In reply to#1342274
Commit-ID:  28a967c3a2f99fa3b5f762f25cb2a319d933571b
Gitweb:     http://git.kernel.org/tip/28a967c3a2f99fa3b5f762f25cb2a319d933571b
Author:     Peter Zijlstra <peterz@infradead.org>
AuthorDate: Wed, 24 Feb 2016 18:45:46 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 25 Feb 2016 08:42:34 +0100

perf: Cure event->pending_disable race

Because event_sched_out() checks event->pending_disable _before_
actually disabling the event, it can happen that the event fires after
it checks but before it gets disabled.

This would leave event->pending_disable set and the queued irq_work
will try and process it.

However, if the event trigger was during schedule(), the event might
have been de-scheduled by the time the irq_work runs, and
perf_event_disable_local() will fail.

Fix this by checking event->pending_disable _after_ we call
event->pmu->del(). This depends on the latter being a compiler
barrier, such that the compiler does not lift the load and re-creates
the problem.

Tested-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: dvyukov@google.com
Cc: eranian@google.com
Cc: oleg@redhat.com
Cc: panand@redhat.com
Cc: sasha.levin@oracle.com
Cc: vince@deater.net
Link: http://lkml.kernel.org/r/20160224174948.040469884@infradead.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index ea064ca..de14b67 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1696,14 +1696,14 @@ event_sched_out(struct perf_event *event,
 
 	perf_pmu_disable(event->pmu);
 
+	event->tstamp_stopped = tstamp;
+	event->pmu->del(event, 0);
+	event->oncpu = -1;
 	event->state = PERF_EVENT_STATE_INACTIVE;
 	if (event->pending_disable) {
 		event->pending_disable = 0;
 		event->state = PERF_EVENT_STATE_OFF;
 	}
-	event->tstamp_stopped = tstamp;
-	event->pmu->del(event, 0);
-	event->oncpu = -1;
 
 	if (!is_software_event(event))
 		cpuctx->active_oncpu--;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web