perf: core, add group scheduling transactional APIs

RFC PATCH In review Lin Ming 3 participants 7 messages Apr 23, 2010 – May 11, 2010

Subsystems (2)

PowerPC perf events

lore

Share & tools

lore

Lin Ming

author

2010-04-23 05:55 UTC #0 lore raw

RFC PATCH v2 1/4 perf: core, add group scheduling transactional APIs

Add group scheduling transactional APIs to struct pmu. These APIs will be implemented in arch code, based on Peter's idea as below.

Quoted 2 lines

the idea behind hw_perf_group_sched_in() is to not perform schedulability tests on each event in the group, but to add the group

Quoted 9 lines

a whole and then perform one test.
Of course, when that test fails, you'll have to roll-back the whole group again.
So start_txn (or a better name) would simply toggle a flag in the pmu implementation that will make pmu::enable() not perform the schedulablilty test.

Quoted 8 lines

Then commit_txn() will perform the schedulability test (so note the method has to have a !void return value, my mistake in the earlier email).
This will allow us to use the regular kernel/perf_event.c::group_sched_in() and all the rollback code. Currently each hw_perf_group_sched_in() implementation duplicates all the rolllback code (with various bugs).

*_txn was renamed to *_group_trans for better readability.

->start_group_trans:
Start group events scheduling transaction, set a flag to make pmu::enable() not perform the schedulability test, it will be performed at commit time.

->commit_group_trans:
Commit group events scheduling transaction, perform the group schedulability as a whole

->stop_group_trans:
Stop group events scheduling transaction, clear the flag so pmu::enable() will perform the schedulability test.

Reviewed-by Stephane Eranian <[email protected]>

Reviewed-by Peter Zijlstra <[email protected]>

Reviewed-by Frederic Weisbecker <[email protected]>

Signed-off-by Lin Ming <[email protected]>

---
 include/linux/perf_event.h |   20 +++++++++++++++++---
 kernel/perf_event.c        |   33 ++++++++++++++++++++-------------
 2 files changed, 37 insertions(+), 16 deletions(-)

include/linux/perf_event.h +17 -3

index ace31fb..f514825 100644
@@ -532,6 +532,8 @@ struct hw_perf_event {
 
 struct perf_event;
 
+#define PERF_EVENT_TRAN_STARTED 1
+
 /**
  * struct pmu - generic performance monitoring unit
  */

@@ -542,6 +544,21 @@ struct pmu {
 	void (*stop)			(struct perf_event *event);
 	void (*read)			(struct perf_event *event);
 	void (*unthrottle)		(struct perf_event *event);
+
+	/*
+	 * group events scheduling is treated as a transaction,
+	 * add group events as a whole and perform one schedulability test.
+	 * If test fails, roll back the whole group
+	 */
+
+	/* start group events transaction  */
+	void (*start_group_trans)	(const struct pmu *pmu);
+
+	/* stop group events transaction  */
+	void (*stop_group_trans)	(const struct pmu *pmu);
+
+	/* commit group events transaction */
+	int (*commit_group_trans)	(const struct pmu *pmu);
 };
 
 /**

@@ -807,9 +824,6 @@ extern void perf_disable(void);
 extern void perf_enable(void);
 extern int perf_event_task_disable(void);
 extern int perf_event_task_enable(void);
-extern int hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx);
 extern void perf_event_update_userpage(struct perf_event *event);
 extern int perf_event_release_kernel(struct perf_event *event);
 extern struct perf_event *

kernel/perf_event.c +20 ~~-13~~

index 9dbe8cd..eac6fe7 100644
@@ -83,14 +83,6 @@ extern __weak const struct pmu *hw_perf_event_init(struct perf_event *event)
 void __weak hw_perf_disable(void)		{ barrier(); }
 void __weak hw_perf_enable(void)		{ barrier(); }
 
-int __weak
-hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx)
-{
-	return 0;
-}
-
 void __weak perf_event_print_debug(void)	{ }
 
 static DEFINE_PER_CPU(int, perf_disable_count);

@@ -641,15 +633,20 @@ group_sched_in(struct perf_event *group_event,
 	       struct perf_cpu_context *cpuctx,
 	       struct perf_event_context *ctx)
 {
-	struct perf_event *event, *partial_group;
+	struct perf_event *event, *partial_group = NULL;
+	const struct pmu *pmu = group_event->pmu;
+	bool trans = false;
 	int ret;
 
 	if (group_event->state == PERF_EVENT_STATE_OFF)
 		return 0;
 
-	ret = hw_perf_group_sched_in(group_event, cpuctx, ctx);
-	if (ret)
-		return ret < 0 ? ret : 0;
+	/* Check if group transaction availabe */
+	if (pmu->start_group_trans)
+		trans = true;
+
+	if (trans)
+		pmu->start_group_trans(pmu);
 
 	if (event_sched_in(group_event, cpuctx, ctx))
 		return -EAGAIN;

@@ -664,9 +661,19 @@ group_sched_in(struct perf_event *group_event,
 		}
 	}
 
-	return 0;
+	if (trans) {
+		ret = pmu->commit_group_trans(pmu);
+		if (!ret) {
+			pmu->stop_group_trans(pmu);
+
+			return 0;
+		}
+	}
 
 group_error:
+	if (trans)
+		pmu->stop_group_trans(pmu);
+
 	/*
 	 * Groups can be scheduled in as one unit only, so undo any
 	 * partial group before returning:

tip-bot for Lin Ming

author

2010-05-07 18:45 UTC #1 lore raw

[tip:perf/core] perf: Add group scheduling transactional APIs

Commit-ID:  6bde9b6ce0127e2a56228a2071536d422be31336
Gitweb:     http://git.kernel.org/tip/6bde9b6ce0127e2a56228a2071536d422be31336
Author:     Lin Ming <[email protected]>
AuthorDate: Fri, 23 Apr 2010 13:56:00 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 7 May 2010 11:31:02 +0200

perf: Add group scheduling transactional APIs

Add group scheduling transactional APIs to struct pmu. These APIs will be implemented in arch code, based on Peter's idea as below.

Quoted 11 lines

the idea behind hw_perf_group_sched_in() is to not perform schedulability tests on each event in the group, but to add the group as a whole and then perform one test.
Of course, when that test fails, you'll have to roll-back the whole group again.
So start_txn (or a better name) would simply toggle a flag in the pmu implementation that will make pmu::enable() not perform the schedulablilty test.

Quoted 7 lines

Then commit_txn() will perform the schedulability test (so note the method has to have a !void return value.
This will allow us to use the regular kernel/perf_event.c::group_sched_in() and all the rollback code. Currently each hw_perf_group_sched_in() implementation duplicates all the rolllback code (with various bugs).

->start_txn:
Start group events scheduling transaction, set a flag to make pmu::enable() not perform the schedulability test, it will be performed at commit time.

->commit_txn:
Commit group events scheduling transaction, perform the group schedulability as a whole

->cancel_txn:
Stop group events scheduling transaction, clear the flag so pmu::enable() will perform the schedulability test.

Reviewed-by Stephane Eranian <[email protected]>

Reviewed-by Frederic Weisbecker <[email protected]>

Signed-off-by Lin Ming <[email protected]>

Cc David Miller <[email protected]>

Cc Paul Mackerras <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

---
 include/linux/perf_event.h |   15 ++++++++++++---
 kernel/perf_event.c        |   33 ++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 16 deletions(-)

include/linux/perf_event.h +12 -3

index 23cd005..4924c96 100644
@@ -547,6 +547,8 @@ struct hw_perf_event {
 
 struct perf_event;
 
+#define PERF_EVENT_TXN_STARTED 1
+
 /**
  * struct pmu - generic performance monitoring unit
  */

@@ -557,6 +559,16 @@ struct pmu {
 	void (*stop)			(struct perf_event *event);
 	void (*read)			(struct perf_event *event);
 	void (*unthrottle)		(struct perf_event *event);
+
+	/*
+	 * group events scheduling is treated as a transaction,
+	 * add group events as a whole and perform one schedulability test.
+	 * If test fails, roll back the whole group
+	 */
+
+	void (*start_txn)	(const struct pmu *pmu);
+	void (*cancel_txn)	(const struct pmu *pmu);
+	int  (*commit_txn)	(const struct pmu *pmu);
 };
 
 /**

@@ -823,9 +835,6 @@ extern void perf_disable(void);
 extern void perf_enable(void);
 extern int perf_event_task_disable(void);
 extern int perf_event_task_enable(void);
-extern int hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx);
 extern void perf_event_update_userpage(struct perf_event *event);
 extern int perf_event_release_kernel(struct perf_event *event);
 extern struct perf_event *

kernel/perf_event.c +20 ~~-13~~

index 34659d4..bb06382 100644
@@ -83,14 +83,6 @@ extern __weak const struct pmu *hw_perf_event_init(struct perf_event *event)
 void __weak hw_perf_disable(void)		{ barrier(); }
 void __weak hw_perf_enable(void)		{ barrier(); }
 
-int __weak
-hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx)
-{
-	return 0;
-}
-
 void __weak perf_event_print_debug(void)	{ }
 
 static DEFINE_PER_CPU(int, perf_disable_count);

@@ -644,15 +636,20 @@ group_sched_in(struct perf_event *group_event,
 	       struct perf_cpu_context *cpuctx,
 	       struct perf_event_context *ctx)
 {
-	struct perf_event *event, *partial_group;
+	struct perf_event *event, *partial_group = NULL;
+	const struct pmu *pmu = group_event->pmu;
+	bool txn = false;
 	int ret;
 
 	if (group_event->state == PERF_EVENT_STATE_OFF)
 		return 0;
 
-	ret = hw_perf_group_sched_in(group_event, cpuctx, ctx);
-	if (ret)
-		return ret < 0 ? ret : 0;
+	/* Check if group transaction availabe */
+	if (pmu->start_txn)
+		txn = true;
+
+	if (txn)
+		pmu->start_txn(pmu);
 
 	if (event_sched_in(group_event, cpuctx, ctx))
 		return -EAGAIN;

@@ -667,9 +664,19 @@ group_sched_in(struct perf_event *group_event,
 		}
 	}
 
-	return 0;
+	if (txn) {
+		ret = pmu->commit_txn(pmu);
+		if (!ret) {
+			pmu->cancel_txn(pmu);
+
+			return 0;
+		}
+	}
 
 group_error:
+	if (txn)
+		pmu->cancel_txn(pmu);
+
 	/*
 	 * Groups can be scheduled in as one unit only, so undo any
 	 * partial group before returning:

Peter Zijlstra

maintainer

2010-05-07 21:56 UTC #2 lore raw

Re: [tip:perf/core] perf: Add group scheduling transactional APIs

Quoted 45 lines · tip-bot for Lin Ming

Commit-ID:  6bde9b6ce0127e2a56228a2071536d422be31336
Gitweb:     http://git.kernel.org/tip/6bde9b6ce0127e2a56228a2071536d422be31336
Author:     Lin Ming <[email protected]>
AuthorDate: Fri, 23 Apr 2010 13:56:00 +0800
Committer:  Ingo Molnar <[email protected]>
CommitDate: Fri, 7 May 2010 11:31:02 +0200
perf: Add group scheduling transactional APIs
Add group scheduling transactional APIs to struct pmu. These APIs will be implemented in arch code, based on Peter's idea as below.
Quoted 11 lines
the idea behind hw_perf_group_sched_in() is to not perform schedulability tests on each event in the group, but to add the group as a whole and then perform one test.
Of course, when that test fails, you'll have to roll-back the whole group again.
So start_txn (or a better name) would simply toggle a flag in the pmu implementation that will make pmu::enable() not perform the schedulablilty test.
Quoted 7 lines
Then commit_txn() will perform the schedulability test (so note the method has to have a !void return value.
This will allow us to use the regular kernel/perf_event.c::group_sched_in() and all the rollback code. Currently each hw_perf_group_sched_in() implementation duplicates all the rolllback code (with various bugs).
->start_txn:
Start group events scheduling transaction, set a flag to make pmu::enable() not perform the schedulability test, it will be performed at commit time.
->commit_txn:
Commit group events scheduling transaction, perform the group schedulability as a whole
->cancel_txn:
Stop group events scheduling transaction, clear the flag so pmu::enable() will perform the schedulability test.

Quoted 8 lines

Reviewed-by Stephane Eranian <[email protected]>
Reviewed-by Frederic Weisbecker <[email protected]>
Signed-off-by Lin Ming <[email protected]>
Cc David Miller <[email protected]>
Cc Paul Mackerras <[email protected]>
Signed-off-by Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>

Paul, are you ok with the powerpc patch that goes with this:
https://patchwork.kernel.org/patch/94620/

(I've got a slightly modified version because I did rename things back to txn)

Dave already said he was OK with the sparc bits.

If you don't object, I'll push both patches to Ingo.

Paul Mackerras

2010-05-08 10:58 UTC #3 lore raw

Re: [tip:perf/core] perf: Add group scheduling transactional APIs

Quoted 5 lines · Peter Zijlstra

Paul, are you ok with the powerpc patch that goes with this:
https://patchwork.kernel.org/patch/94620/
(I've got a slightly modified version because I did rename things back to txn)

That version doesn't update event->hw.config properly; the thing is that some events have multiple event codes that count the same thing but have different constraints (e.g. one code might only work on one hardware counter whereas another might work on any counter but constrain what can be counted on other counters).

So power_check_constraints() searches through the space of possible alternative codes and returns the results in cpuhw->events[]. I then put the alternatives chosen back into each event->hw.config to use as the starting point for the search next time. This means that event->hw.config has to be updated after power_check_constraints() has been called in power_pmu_commit_txn, not in the no-check case in power_pmu_enable().

Here's the patch I ended up with.

--------------
Subject: perf, powerpc: Implement group scheduling transactional APIs
From: Lin Ming <[email protected]>

[[email protected]: Set cpuhw->event[i]->hw.config in power_pmu_commit_txn.]

Signed-off-by Lin Ming <[email protected]>

Signed-off-by Paul Mackerras <[email protected]>

---

arch/powerpc/kernel/perf_event.c +68 ~~-61~~

index 08460a2..43b83c3 100644
@@ -35,6 +35,9 @@ struct cpu_hw_events {
 	u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long amasks[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long avalues[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
+
+	unsigned int group_flag;
+	int n_txn_start;
 };
 DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);

@@ -718,66 +721,6 @@ static int collect_events(struct perf_event *group, int max_count,
 	return n;
 }
 
-static void event_sched_in(struct perf_event *event)
-{
-	event->state = PERF_EVENT_STATE_ACTIVE;
-	event->oncpu = smp_processor_id();
-	event->tstamp_running += event->ctx->time - event->tstamp_stopped;
-	if (is_software_event(event))
-		event->pmu->enable(event);
-}
-
-/*
- * Called to enable a whole group of events.
- * Returns 1 if the group was enabled, or -EAGAIN if it could not be.
- * Assumes the caller has disabled interrupts and has
- * frozen the PMU with hw_perf_save_disable.
- */
-int hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx)
-{
-	struct cpu_hw_events *cpuhw;
-	long i, n, n0;
-	struct perf_event *sub;
-
-	if (!ppmu)
-		return 0;
-	cpuhw = &__get_cpu_var(cpu_hw_events);
-	n0 = cpuhw->n_events;
-	n = collect_events(group_leader, ppmu->n_counter - n0,
-			   &cpuhw->event[n0], &cpuhw->events[n0],
-			   &cpuhw->flags[n0]);
-	if (n < 0)
-		return -EAGAIN;
-	if (check_excludes(cpuhw->event, cpuhw->flags, n0, n))
-		return -EAGAIN;
-	i = power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n + n0);
-	if (i < 0)
-		return -EAGAIN;
-	cpuhw->n_events = n0 + n;
-	cpuhw->n_added += n;
-
-	/*
-	 * OK, this group can go on; update event states etc.,
-	 * and enable any software events
-	 */
-	for (i = n0; i < n0 + n; ++i)
-		cpuhw->event[i]->hw.config = cpuhw->events[i];
-	cpuctx->active_oncpu += n;
-	n = 1;
-	event_sched_in(group_leader);
-	list_for_each_entry(sub, &group_leader->sibling_list, group_entry) {
-		if (sub->state != PERF_EVENT_STATE_OFF) {
-			event_sched_in(sub);
-			++n;
-		}
-	}
-	ctx->nr_active += n;
-
-	return 1;
-}
-
 /*
  * Add a event to the PMU.
  * If all events are not already frozen, then we disable and

@@ -805,12 +748,22 @@ static int power_pmu_enable(struct perf_event *event)
 	cpuhw->event[n0] = event;
 	cpuhw->events[n0] = event->hw.config;
 	cpuhw->flags[n0] = event->hw.event_base;
+
+	/*
+	 * If group events scheduling transaction was started,
+	 * skip the schedulability test here, it will be peformed
+	 * at commit time(->commit_txn) as a whole
+	 */
+	if (cpuhw->group_flag & PERF_EVENT_TXN_STARTED)
+		goto nocheck;
+
 	if (check_excludes(cpuhw->event, cpuhw->flags, n0, 1))
 		goto out;
 	if (power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n0 + 1))
 		goto out;
-
 	event->hw.config = cpuhw->events[n0];
+
+nocheck:
 	++cpuhw->n_events;
 	++cpuhw->n_added;

@@ -896,11 +849,65 @@ static void power_pmu_unthrottle(struct perf_event *event)
 	local_irq_restore(flags);
 }
 
+/*
+ * Start group events scheduling transaction
+ * Set the flag to make pmu::enable() not perform the
+ * schedulability test, it will be performed at commit time
+ */
+void power_pmu_start_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag |= PERF_EVENT_TXN_STARTED;
+	cpuhw->n_txn_start = cpuhw->n_events;
+}
+
+/*
+ * Stop group events scheduling transaction
+ * Clear the flag and pmu::enable() will perform the
+ * schedulability test.
+ */
+void power_pmu_cancel_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag &= ~PERF_EVENT_TXN_STARTED;
+}
+
+/*
+ * Commit group events scheduling transaction
+ * Perform the group schedulability test as a whole
+ * Return 0 if success
+ */
+int power_pmu_commit_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw;
+	long i, n;
+
+	if (!ppmu)
+		return -EAGAIN;
+	cpuhw = &__get_cpu_var(cpu_hw_events);
+	n = cpuhw->n_events;
+	if (check_excludes(cpuhw->event, cpuhw->flags, 0, n))
+		return -EAGAIN;
+	i = power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n);
+	if (i < 0)
+		return -EAGAIN;
+
+	for (i = cpuhw->n_txn_start; i < n; ++i)
+		cpuhw->event[i]->hw.config = cpuhw->events[i];
+
+	return 0;
+}
+
 struct pmu power_pmu = {
 	.enable		= power_pmu_enable,
 	.disable	= power_pmu_disable,
 	.read		= power_pmu_read,
 	.unthrottle	= power_pmu_unthrottle,
+	.start_txn	= power_pmu_start_txn,
+	.cancel_txn	= power_pmu_cancel_txn,
+	.commit_txn	= power_pmu_commit_txn,
 };
 
 /*

Peter Zijlstra

maintainer

2010-05-10 08:12 UTC #4 lore raw

Re: [tip:perf/core] perf: Add group scheduling transactional APIs

Quoted 1 line · Paul Mackerras

Here's the patch I ended up with.

Ok, I'll take this one. Thanks Paul!

One small question on the patch below:

Quoted 91 lines

--------------
Subject: perf, powerpc: Implement group scheduling transactional APIs
From: Lin Ming <[email protected]>

[[email protected]: Set cpuhw->event[i]->hw.config in power_pmu_commit_txn.]

Signed-off-by Lin Ming <[email protected]>

Signed-off-by Paul Mackerras <[email protected]>

---

arch/powerpc/kernel/perf_event.c +54 -0

index 08460a2..43b83c3 100644
@@ -35,6 +35,9 @@ struct cpu_hw_events {
 	u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long amasks[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long avalues[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
+
+	unsigned int group_flag;
+	int n_txn_start;
 };
 DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);

@@ -805,12 +748,22 @@ static int power_pmu_enable(struct perf_event *event)
 	cpuhw->event[n0] = event;
 	cpuhw->events[n0] = event->hw.config;
 	cpuhw->flags[n0] = event->hw.event_base;
+
+	/*
+	 * If group events scheduling transaction was started,
+	 * skip the schedulability test here, it will be peformed
+	 * at commit time(->commit_txn) as a whole
+	 */
+	if (cpuhw->group_flag & PERF_EVENT_TXN_STARTED)
+		goto nocheck;
+
 	if (check_excludes(cpuhw->event, cpuhw->flags, n0, 1))
 		goto out;
 	if (power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n0 + 1))
 		goto out;
 	event->hw.config = cpuhw->events[n0];
+
+nocheck:
 	++cpuhw->n_events;
 	++cpuhw->n_added;

@@ -896,11 +849,65 @@ static void power_pmu_unthrottle(struct perf_event *event)
 	local_irq_restore(flags);
 }
 
+/*
+ * Start group events scheduling transaction
+ * Set the flag to make pmu::enable() not perform the
+ * schedulability test, it will be performed at commit time
+ */
+void power_pmu_start_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag |= PERF_EVENT_TXN_STARTED;
+	cpuhw->n_txn_start = cpuhw->n_events;
+}
+
+/*
+ * Stop group events scheduling transaction
+ * Clear the flag and pmu::enable() will perform the
+ * schedulability test.
+ */
+void power_pmu_cancel_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag &= ~PERF_EVENT_TXN_STARTED;
+}
+
+/*
+ * Commit group events scheduling transaction
+ * Perform the group schedulability test as a whole
+ * Return 0 if success
+ */
+int power_pmu_commit_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw;
+	long i, n;
+
+	if (!ppmu)
+		return -EAGAIN;
+	cpuhw = &__get_cpu_var(cpu_hw_events);
+	n = cpuhw->n_events;
+	if (check_excludes(cpuhw->event, cpuhw->flags, 0, n))

arch/powerpc/kernel/perf_event.c static void power_pmu_unthrottle(struct perf_event *event) · line 901

Quoted 10 lines

arch/powerpc/kernel/perf_event.c +10 -0

+		return -EAGAIN;
+	i = power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n);
+	if (i < 0)
+		return -EAGAIN;
+
+	for (i = cpuhw->n_txn_start; i < n; ++i)
+		cpuhw->event[i]->hw.config = cpuhw->events[i];
+
+	return 0;
+}

Can't you compute n_txn_start by subtracting n_added from n_events ?

Paul Mackerras

2010-05-10 11:39 UTC #5 lore raw

Re: [tip:perf/core] perf: Add group scheduling transactional APIs

Quoted 1 line · Peter Zijlstra

Can't you compute n_txn_start by subtracting n_added from n_events ?

Well, n_added says how many have been added since hw_perf_disable, and we could get multiple transactions inside a hw_perf_disable/enable pair, if we're scheduling multiple groups on. So n_events - n_added doesn't tell us how many have been added in this transaction -- unless I'm mistaken and there is a one transaction per hw_perf_disable/enable rule, but I didn't see that written anywhere, and it does seem to be explicitly one transaction per group.

Paul.

tip-bot for Lin Ming

author

2010-05-11 15:44 UTC #6 lore raw

[tip:perf/core] perf, powerpc: Implement group scheduling transactional APIs

In reply to Paul Mackerras

Commit-ID:  8e6d5573af55435160d329f6ae3fe16a0abbdaec
Gitweb:     http://git.kernel.org/tip/8e6d5573af55435160d329f6ae3fe16a0abbdaec
Author:     Lin Ming <[email protected]>
AuthorDate: Sat, 8 May 2010 20:28:41 +1000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 11 May 2010 17:08:24 +0200

perf, powerpc: Implement group scheduling transactional APIs

[[email protected]: Set cpuhw->event[i]->hw.config in power_pmu_commit_txn.]

Signed-off-by Lin Ming <[email protected]>

Signed-off-by Paul Mackerras <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/powerpc/kernel/perf_event.c |  129 ++++++++++++++++++++------------------
 1 files changed, 68 insertions(+), 61 deletions(-)

arch/powerpc/kernel/perf_event.c +68 ~~-61~~

index 08460a2..43b83c3 100644
@@ -35,6 +35,9 @@ struct cpu_hw_events {
 	u64 alternatives[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long amasks[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
 	unsigned long avalues[MAX_HWEVENTS][MAX_EVENT_ALTERNATIVES];
+
+	unsigned int group_flag;
+	int n_txn_start;
 };
 DEFINE_PER_CPU(struct cpu_hw_events, cpu_hw_events);

@@ -718,66 +721,6 @@ static int collect_events(struct perf_event *group, int max_count,
 	return n;
 }
 
-static void event_sched_in(struct perf_event *event)
-{
-	event->state = PERF_EVENT_STATE_ACTIVE;
-	event->oncpu = smp_processor_id();
-	event->tstamp_running += event->ctx->time - event->tstamp_stopped;
-	if (is_software_event(event))
-		event->pmu->enable(event);
-}
-
-/*
- * Called to enable a whole group of events.
- * Returns 1 if the group was enabled, or -EAGAIN if it could not be.
- * Assumes the caller has disabled interrupts and has
- * frozen the PMU with hw_perf_save_disable.
- */
-int hw_perf_group_sched_in(struct perf_event *group_leader,
-	       struct perf_cpu_context *cpuctx,
-	       struct perf_event_context *ctx)
-{
-	struct cpu_hw_events *cpuhw;
-	long i, n, n0;
-	struct perf_event *sub;
-
-	if (!ppmu)
-		return 0;
-	cpuhw = &__get_cpu_var(cpu_hw_events);
-	n0 = cpuhw->n_events;
-	n = collect_events(group_leader, ppmu->n_counter - n0,
-			   &cpuhw->event[n0], &cpuhw->events[n0],
-			   &cpuhw->flags[n0]);
-	if (n < 0)
-		return -EAGAIN;
-	if (check_excludes(cpuhw->event, cpuhw->flags, n0, n))
-		return -EAGAIN;
-	i = power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n + n0);
-	if (i < 0)
-		return -EAGAIN;
-	cpuhw->n_events = n0 + n;
-	cpuhw->n_added += n;
-
-	/*
-	 * OK, this group can go on; update event states etc.,
-	 * and enable any software events
-	 */
-	for (i = n0; i < n0 + n; ++i)
-		cpuhw->event[i]->hw.config = cpuhw->events[i];
-	cpuctx->active_oncpu += n;
-	n = 1;
-	event_sched_in(group_leader);
-	list_for_each_entry(sub, &group_leader->sibling_list, group_entry) {
-		if (sub->state != PERF_EVENT_STATE_OFF) {
-			event_sched_in(sub);
-			++n;
-		}
-	}
-	ctx->nr_active += n;
-
-	return 1;
-}
-
 /*
  * Add a event to the PMU.
  * If all events are not already frozen, then we disable and

@@ -805,12 +748,22 @@ static int power_pmu_enable(struct perf_event *event)
 	cpuhw->event[n0] = event;
 	cpuhw->events[n0] = event->hw.config;
 	cpuhw->flags[n0] = event->hw.event_base;
+
+	/*
+	 * If group events scheduling transaction was started,
+	 * skip the schedulability test here, it will be peformed
+	 * at commit time(->commit_txn) as a whole
+	 */
+	if (cpuhw->group_flag & PERF_EVENT_TXN_STARTED)
+		goto nocheck;
+
 	if (check_excludes(cpuhw->event, cpuhw->flags, n0, 1))
 		goto out;
 	if (power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n0 + 1))
 		goto out;
-
 	event->hw.config = cpuhw->events[n0];
+
+nocheck:
 	++cpuhw->n_events;
 	++cpuhw->n_added;

@@ -896,11 +849,65 @@ static void power_pmu_unthrottle(struct perf_event *event)
 	local_irq_restore(flags);
 }
 
+/*
+ * Start group events scheduling transaction
+ * Set the flag to make pmu::enable() not perform the
+ * schedulability test, it will be performed at commit time
+ */
+void power_pmu_start_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag |= PERF_EVENT_TXN_STARTED;
+	cpuhw->n_txn_start = cpuhw->n_events;
+}
+
+/*
+ * Stop group events scheduling transaction
+ * Clear the flag and pmu::enable() will perform the
+ * schedulability test.
+ */
+void power_pmu_cancel_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw = &__get_cpu_var(cpu_hw_events);
+
+	cpuhw->group_flag &= ~PERF_EVENT_TXN_STARTED;
+}
+
+/*
+ * Commit group events scheduling transaction
+ * Perform the group schedulability test as a whole
+ * Return 0 if success
+ */
+int power_pmu_commit_txn(const struct pmu *pmu)
+{
+	struct cpu_hw_events *cpuhw;
+	long i, n;
+
+	if (!ppmu)
+		return -EAGAIN;
+	cpuhw = &__get_cpu_var(cpu_hw_events);
+	n = cpuhw->n_events;
+	if (check_excludes(cpuhw->event, cpuhw->flags, 0, n))
+		return -EAGAIN;
+	i = power_check_constraints(cpuhw, cpuhw->events, cpuhw->flags, n);
+	if (i < 0)
+		return -EAGAIN;
+
+	for (i = cpuhw->n_txn_start; i < n; ++i)
+		cpuhw->event[i]->hw.config = cpuhw->events[i];
+
+	return 0;
+}
+
 struct pmu power_pmu = {
 	.enable		= power_pmu_enable,
 	.disable	= power_pmu_disable,
 	.read		= power_pmu_read,
 	.unthrottle	= power_pmu_unthrottle,
+	.start_txn	= power_pmu_start_txn,
+	.cancel_txn	= power_pmu_cancel_txn,
+	.commit_txn	= power_pmu_commit_txn,
 };
 
 /*