All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: correct ctx_event_type in ctx_resched()
@ 2018-03-06  5:55 Song Liu
  2018-03-06  8:54 ` Peter Zijlstra
  2018-03-09  9:07 ` [tip:perf/urgent] perf/core: Fix " tip-bot for Song Liu
  0 siblings, 2 replies; 3+ messages in thread
From: Song Liu @ 2018-03-06  5:55 UTC (permalink / raw)
  To: linux-kernel, peterz, jolsa; +Cc: kernel-team, ephiepark, Song Liu

In ctx_resched(), EVENT_FLEXIBLE should be sched_out when EVENT_PINNED is
added. However, ctx_resched() calculates ctx_event_type before checking
this condition. As a result, pinned events will NOT get higher priority
than flexible events.

The following shows this issue on an Intel CPU (where ref-cycles can
only use one hardware counter).

  1. First start:
       perf stat -C 0 -e ref-cycles  -I 1000
  2. Then, in the second console, run:
       perf stat -C 0 -e ref-cycles:D -I 1000

The second perf uses pinned events, which is expected to have higher
priority. However, because it failed in ctx_resched(). It is never
run.

This patch fixes this by calculating ctx_event_type after re-evaluating
event_type.

Fixes: 487f05e18aa4 ("perf/core: Optimize event rescheduling on active contexts")
Signed-off-by: Song Liu <songliubraving@fb.com>
Reported-by: Ephraim Park <ephiepark@fb.com>
---
 kernel/events/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5789810..cf52fc0 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2246,7 +2246,7 @@ static void ctx_resched(struct perf_cpu_context *cpuctx,
 			struct perf_event_context *task_ctx,
 			enum event_type_t event_type)
 {
-	enum event_type_t ctx_event_type = event_type & EVENT_ALL;
+	enum event_type_t ctx_event_type;
 	bool cpu_event = !!(event_type & EVENT_CPU);
 
 	/*
@@ -2256,6 +2256,8 @@ static void ctx_resched(struct perf_cpu_context *cpuctx,
 	if (event_type & EVENT_PINNED)
 		event_type |= EVENT_FLEXIBLE;
 
+	ctx_event_type = event_type & EVENT_ALL;
+
 	perf_pmu_disable(cpuctx->ctx.pmu);
 	if (task_ctx)
 		task_ctx_sched_out(cpuctx, task_ctx, event_type);
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] perf: correct ctx_event_type in ctx_resched()
  2018-03-06  5:55 [PATCH] perf: correct ctx_event_type in ctx_resched() Song Liu
@ 2018-03-06  8:54 ` Peter Zijlstra
  2018-03-09  9:07 ` [tip:perf/urgent] perf/core: Fix " tip-bot for Song Liu
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Zijlstra @ 2018-03-06  8:54 UTC (permalink / raw)
  To: Song Liu; +Cc: linux-kernel, jolsa, kernel-team, ephiepark

On Mon, Mar 05, 2018 at 09:55:04PM -0800, Song Liu wrote:
> In ctx_resched(), EVENT_FLEXIBLE should be sched_out when EVENT_PINNED is
> added. However, ctx_resched() calculates ctx_event_type before checking
> this condition. As a result, pinned events will NOT get higher priority
> than flexible events.
> 
> The following shows this issue on an Intel CPU (where ref-cycles can
> only use one hardware counter).
> 
>   1. First start:
>        perf stat -C 0 -e ref-cycles  -I 1000
>   2. Then, in the second console, run:
>        perf stat -C 0 -e ref-cycles:D -I 1000
> 
> The second perf uses pinned events, which is expected to have higher
> priority. However, because it failed in ctx_resched(). It is never
> run.
> 
> This patch fixes this by calculating ctx_event_type after re-evaluating
> event_type.
> 
> Fixes: 487f05e18aa4 ("perf/core: Optimize event rescheduling on active contexts")
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Reported-by: Ephraim Park <ephiepark@fb.com>

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread
* [tip:perf/urgent] perf/core: Fix ctx_event_type in ctx_resched()
  2018-03-06  5:55 [PATCH] perf: correct ctx_event_type in ctx_resched() Song Liu
  2018-03-06  8:54 ` Peter Zijlstra
@ 2018-03-09  9:07 ` tip-bot for Song Liu
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Song Liu @ 2018-03-09  9:07 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, tglx, torvalds, linux-kernel, hpa, ephiepark,
	vincent.weaver, songliubraving, alexander.shishkin, peterz,
	kernel-team, jolsa, acme, eranian

Commit-ID:  bd903afeb504db5655a45bb4cf86f38be5b1bf62
Gitweb:     https://git.kernel.org/tip/bd903afeb504db5655a45bb4cf86f38be5b1bf62
Author:     Song Liu <songliubraving@fb.com>
AuthorDate: Mon, 5 Mar 2018 21:55:04 -0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 9 Mar 2018 08:03:02 +0100

perf/core: Fix ctx_event_type in ctx_resched()

In ctx_resched(), EVENT_FLEXIBLE should be sched_out when EVENT_PINNED is
added. However, ctx_resched() calculates ctx_event_type before checking
this condition. As a result, pinned events will NOT get higher priority
than flexible events.

The following shows this issue on an Intel CPU (where ref-cycles can
only use one hardware counter).

  1. First start:
       perf stat -C 0 -e ref-cycles  -I 1000
  2. Then, in the second console, run:
       perf stat -C 0 -e ref-cycles:D -I 1000

The second perf uses pinned events, which is expected to have higher
priority. However, because it failed in ctx_resched(). It is never
run.

This patch fixes this by calculating ctx_event_type after re-evaluating
event_type.

Reported-by: Ephraim Park <ephiepark@fb.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <jolsa@redhat.com>
Cc: <kernel-team@fb.com>
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: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Fixes: 487f05e18aa4 ("perf/core: Optimize event rescheduling on active contexts")
Link: http://lkml.kernel.org/r/20180306055504.3283731-1-songliubraving@fb.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 96db9ae5d5af..4b838470fac4 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2246,7 +2246,7 @@ static void ctx_resched(struct perf_cpu_context *cpuctx,
 			struct perf_event_context *task_ctx,
 			enum event_type_t event_type)
 {
-	enum event_type_t ctx_event_type = event_type & EVENT_ALL;
+	enum event_type_t ctx_event_type;
 	bool cpu_event = !!(event_type & EVENT_CPU);
 
 	/*
@@ -2256,6 +2256,8 @@ static void ctx_resched(struct perf_cpu_context *cpuctx,
 	if (event_type & EVENT_PINNED)
 		event_type |= EVENT_FLEXIBLE;
 
+	ctx_event_type = event_type & EVENT_ALL;
+
 	perf_pmu_disable(cpuctx->ctx.pmu);
 	if (task_ctx)
 		task_ctx_sched_out(cpuctx, task_ctx, event_type);

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-09  9:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-06  5:55 [PATCH] perf: correct ctx_event_type in ctx_resched() Song Liu
2018-03-06  8:54 ` Peter Zijlstra
2018-03-09  9:07 ` [tip:perf/urgent] perf/core: Fix " tip-bot for Song Liu

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.