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


Groups > linux.kernel > #1450703 > unrolled thread

[PATCH] perf: sched out groups atomically

Started byMark Rutland <mark.rutland@arm.com>
First post2016-07-26 19:20 +0200
Last post2016-08-10 21:50 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] perf: sched out groups atomically Mark Rutland <mark.rutland@arm.com> - 2016-07-26 19:20 +0200
    Re: [PATCH] perf: sched out groups atomically Peter Zijlstra <peterz@infradead.org> - 2016-08-08 14:20 +0200
    [tip:perf/core] perf/core: Sched out groups atomically tip-bot for Mark Rutland <tipbot@zytor.com> - 2016-08-10 21:50 +0200

#1450703 — [PATCH] perf: sched out groups atomically

FromMark Rutland <mark.rutland@arm.com>
Date2016-07-26 19:20 +0200
Subject[PATCH] perf: sched out groups atomically
Message-ID<rZebn-4SB-11@gated-at.bofh.it>
Groups of events are supposed to be scheduled atomically, such that it
is possible to derive meaningful ratios between their values.

We take great pains to achieve this when scheduling event groups to a
PMU in group_sched_in(), calling {start,commit}_txn() (which fall back
to perf_pmu_{disable,enable}() if necessary) to provide this guarantee.
However we don't mirror this in group_sched_out(), and in some cases
events will not be scheduled out atomically.

For example, if we disable an event group with PERF_EVENT_IOC_DISABLE,
we'll cross-call __perf_event_disable() for the group leader, and will
call group_sched_out() without having first disabled the relevant PMU.
We will disable/enable the PMU around each pmu->del() call, but between
each call the PMU will be enabled and events may count.

Avoid this by explicitly disabling and enabling the PMU around event
removal in group_sched_out(), mirroring what we do in group_sched_in().

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
---
 kernel/events/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 68cac68..c4a0ec3 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1770,6 +1770,8 @@ group_sched_out(struct perf_event *group_event,
 	struct perf_event *event;
 	int state = group_event->state;
 
+	perf_pmu_disable(ctx->pmu);
+
 	event_sched_out(group_event, cpuctx, ctx);
 
 	/*
@@ -1778,6 +1780,8 @@ group_sched_out(struct perf_event *group_event,
 	list_for_each_entry(event, &group_event->sibling_list, group_entry)
 		event_sched_out(event, cpuctx, ctx);
 
+	perf_pmu_enable(ctx->pmu);
+
 	if (state == PERF_EVENT_STATE_ACTIVE && group_event->attr.exclusive)
 		cpuctx->exclusive = 0;
 }
-- 
1.9.1

[toc] | [next] | [standalone]


#1457755

FromPeter Zijlstra <peterz@infradead.org>
Date2016-08-08 14:20 +0200
Message-ID<s3RHb-1ii-17@gated-at.bofh.it>
In reply to#1450703
On Tue, Jul 26, 2016 at 06:12:21PM +0100, Mark Rutland wrote:
> Groups of events are supposed to be scheduled atomically, such that it
> is possible to derive meaningful ratios between their values.
> 
> We take great pains to achieve this when scheduling event groups to a
> PMU in group_sched_in(), calling {start,commit}_txn() (which fall back
> to perf_pmu_{disable,enable}() if necessary) to provide this guarantee.
> However we don't mirror this in group_sched_out(), and in some cases
> events will not be scheduled out atomically.
> 
> For example, if we disable an event group with PERF_EVENT_IOC_DISABLE,
> we'll cross-call __perf_event_disable() for the group leader, and will
> call group_sched_out() without having first disabled the relevant PMU.
> We will disable/enable the PMU around each pmu->del() call, but between
> each call the PMU will be enabled and events may count.
> 
> Avoid this by explicitly disabling and enabling the PMU around event
> removal in group_sched_out(), mirroring what we do in group_sched_in().

Thanks!

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


#1459709 — [tip:perf/core] perf/core: Sched out groups atomically

Fromtip-bot for Mark Rutland <tipbot@zytor.com>
Date2016-08-10 21:50 +0200
Subject[tip:perf/core] perf/core: Sched out groups atomically
Message-ID<s4HFM-12V-25@gated-at.bofh.it>
In reply to#1450703
Commit-ID:  3f005e7de3db8d0b3f7a1f399aa061dc35b65864
Gitweb:     http://git.kernel.org/tip/3f005e7de3db8d0b3f7a1f399aa061dc35b65864
Author:     Mark Rutland <mark.rutland@arm.com>
AuthorDate: Tue, 26 Jul 2016 18:12:21 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 10 Aug 2016 13:13:23 +0200

perf/core: Sched out groups atomically

Groups of events are supposed to be scheduled atomically, such that it
is possible to derive meaningful ratios between their values.

We take great pains to achieve this when scheduling event groups to a
PMU in group_sched_in(), calling {start,commit}_txn() (which fall back
to perf_pmu_{disable,enable}() if necessary) to provide this guarantee.
However we don't mirror this in group_sched_out(), and in some cases
events will not be scheduled out atomically.

For example, if we disable an event group with PERF_EVENT_IOC_DISABLE,
we'll cross-call __perf_event_disable() for the group leader, and will
call group_sched_out() without having first disabled the relevant PMU.
We will disable/enable the PMU around each pmu->del() call, but between
each call the PMU will be enabled and events may count.

Avoid this by explicitly disabling and enabling the PMU around event
removal in group_sched_out(), mirroring what we do in group_sched_in().

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
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: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1469553141-28314-1-git-send-email-mark.rutland@arm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1903b8f..11f6bbe 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1796,6 +1796,8 @@ group_sched_out(struct perf_event *group_event,
 	struct perf_event *event;
 	int state = group_event->state;
 
+	perf_pmu_disable(ctx->pmu);
+
 	event_sched_out(group_event, cpuctx, ctx);
 
 	/*
@@ -1804,6 +1806,8 @@ group_sched_out(struct perf_event *group_event,
 	list_for_each_entry(event, &group_event->sibling_list, group_entry)
 		event_sched_out(event, cpuctx, ctx);
 
+	perf_pmu_enable(ctx->pmu);
+
 	if (state == PERF_EVENT_STATE_ACTIVE && group_event->attr.exclusive)
 		cpuctx->exclusive = 0;
 }

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web