LKML Archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH v2 1/4] perf: core, add group scheduling transactional APIs
@ 2010-04-23  5:56 Lin Ming
  2010-05-07 18:44 ` [tip:perf/core] perf: Add " tip-bot for Lin Ming
  0 siblings, 1 reply; 7+ messages in thread
From: Lin Ming @ 2010-04-23  5:56 UTC (permalink / raw
  To: Peter Zijlstra, Frederic Weisbecker, Ingo Molnar,
	eranian@gmail.com, Gary.Mohr@Bull.com, Corey Ashford,
	arjan@linux.intel.com, Zhang, Yanmin, Paul Mackerras,
	David S. Miller
  Cc: lkml

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

> 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.
>
> 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 <eranian@google.com>
Reviewed-by: Peter Zijlstra <peterz@infradead.org>
Reviewed-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>

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

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index ace31fb..f514825 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -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 *
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 9dbe8cd..eac6fe7 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -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:




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [tip:perf/core] perf: Add group scheduling transactional APIs
  2010-04-23  5:56 [RFC][PATCH v2 1/4] perf: core, add group scheduling transactional APIs Lin Ming
@ 2010-05-07 18:44 ` tip-bot for Lin Ming
  2010-05-07 21:55   ` Peter Zijlstra
  0 siblings, 1 reply; 7+ messages in thread
From: tip-bot for Lin Ming @ 2010-05-07 18:44 UTC (permalink / raw
  To: linux-tip-commits
  Cc: linux-kernel, paulus, eranian, hpa, mingo, a.p.zijlstra, davem,
	fweisbec, ming.m.lin, tglx, mingo

Commit-ID:  6bde9b6ce0127e2a56228a2071536d422be31336
Gitweb:     http://git.kernel.org/tip/6bde9b6ce0127e2a56228a2071536d422be31336
Author:     Lin Ming <ming.m.lin@intel.com>
AuthorDate: Fri, 23 Apr 2010 13:56:00 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
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.

> 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.
>
> 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 <eranian@google.com>
Reviewed-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Lin Ming <ming.m.lin@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1272002160.5707.60.camel@minggr.sh.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/perf_event.h |   15 ++++++++++++---
 kernel/perf_event.c        |   33 ++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 23cd005..4924c96 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -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 *
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 34659d4..bb06382 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -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:

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [tip:perf/core] perf: Add group scheduling transactional APIs
  2010-05-07 18:44 ` [tip:perf/core] perf: Add " tip-bot for Lin Ming
@ 2010-05-07 21:55   ` Peter Zijlstra
  2010-05-08 10:28     ` Paul Mackerras
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2010-05-07 21:55 UTC (permalink / raw
  To: mingo, hpa, eranian, paulus, linux-kernel, davem, fweisbec,
	ming.m.lin, tglx, mingo
  Cc: linux-tip-commits

On Fri, 2010-05-07 at 18:44 +0000, tip-bot for Lin Ming wrote:
> Commit-ID:  6bde9b6ce0127e2a56228a2071536d422be31336
> Gitweb:     http://git.kernel.org/tip/6bde9b6ce0127e2a56228a2071536d422be31336
> Author:     Lin Ming <ming.m.lin@intel.com>
> AuthorDate: Fri, 23 Apr 2010 13:56:00 +0800
> Committer:  Ingo Molnar <mingo@elte.hu>
> 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.
> 
> > 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.
> >
> > 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 <eranian@google.com>
> Reviewed-by: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> Cc: David Miller <davem@davemloft.net>
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> LKML-Reference: <1272002160.5707.60.camel@minggr.sh.intel.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu> 


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.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [tip:perf/core] perf: Add group scheduling transactional APIs
  2010-05-07 21:55   ` Peter Zijlstra
@ 2010-05-08 10:28     ` Paul Mackerras
  2010-05-10  8:11       ` Peter Zijlstra
  2010-05-11 15:43       ` [tip:perf/core] perf, powerpc: Implement " tip-bot for Lin Ming
  0 siblings, 2 replies; 7+ messages in thread
From: Paul Mackerras @ 2010-05-08 10:28 UTC (permalink / raw
  To: Peter Zijlstra
  Cc: mingo, hpa, eranian, linux-kernel, davem, fweisbec, ming.m.lin,
	tglx, mingo, linux-tip-commits

On Fri, May 07, 2010 at 11:55:55PM +0200, Peter Zijlstra wrote:

> 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 <ming.m.lin@intel.com>

[paulus@samba.org: Set cpuhw->event[i]->hw.config in
power_pmu_commit_txn.]

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/perf_event.c
index 08460a2..43b83c3 100644
--- a/arch/powerpc/kernel/perf_event.c
+++ b/arch/powerpc/kernel/perf_event.c
@@ -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,
 };
 
 /*

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [tip:perf/core] perf: Add group scheduling transactional APIs
  2010-05-08 10:28     ` Paul Mackerras
@ 2010-05-10  8:11       ` Peter Zijlstra
  2010-05-10 11:38         ` Paul Mackerras
  2010-05-11 15:43       ` [tip:perf/core] perf, powerpc: Implement " tip-bot for Lin Ming
  1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2010-05-10  8:11 UTC (permalink / raw
  To: Paul Mackerras
  Cc: mingo, hpa, eranian, linux-kernel, davem, fweisbec, ming.m.lin,
	tglx, mingo, linux-tip-commits

On Sat, 2010-05-08 at 20:28 +1000, Paul Mackerras wrote:

> Here's the patch I ended up with.

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

One small question on the patch below:

> --------------
> Subject: perf, powerpc: Implement group scheduling transactional APIs
> From: Lin Ming <ming.m.lin@intel.com>
> 
> [paulus@samba.org: Set cpuhw->event[i]->hw.config in
> power_pmu_commit_txn.]
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
> diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/perf_event.c
> index 08460a2..43b83c3 100644
> --- a/arch/powerpc/kernel/perf_event.c
> +++ b/arch/powerpc/kernel/perf_event.c
> @@ -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))
> +		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 ?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [tip:perf/core] perf: Add group scheduling transactional APIs
  2010-05-10  8:11       ` Peter Zijlstra
@ 2010-05-10 11:38         ` Paul Mackerras
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Mackerras @ 2010-05-10 11:38 UTC (permalink / raw
  To: Peter Zijlstra
  Cc: mingo, hpa, eranian, linux-kernel, davem, fweisbec, ming.m.lin,
	tglx, mingo, linux-tip-commits

On Mon, May 10, 2010 at 10:11:55AM +0200, Peter Zijlstra wrote:

> 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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tip:perf/core] perf, powerpc: Implement group scheduling transactional APIs
  2010-05-08 10:28     ` Paul Mackerras
  2010-05-10  8:11       ` Peter Zijlstra
@ 2010-05-11 15:43       ` tip-bot for Lin Ming
  1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Lin Ming @ 2010-05-11 15:43 UTC (permalink / raw
  To: linux-tip-commits
  Cc: linux-kernel, paulus, hpa, mingo, a.p.zijlstra, ming.m.lin, tglx,
	mingo

Commit-ID:  8e6d5573af55435160d329f6ae3fe16a0abbdaec
Gitweb:     http://git.kernel.org/tip/8e6d5573af55435160d329f6ae3fe16a0abbdaec
Author:     Lin Ming <ming.m.lin@intel.com>
AuthorDate: Sat, 8 May 2010 20:28:41 +1000
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 11 May 2010 17:08:24 +0200

perf, powerpc: Implement group scheduling transactional APIs

[paulus@samba.org: Set cpuhw->event[i]->hw.config in
power_pmu_commit_txn.]

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <20100508102841.GA10650@brick.ozlabs.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/powerpc/kernel/perf_event.c |  129 ++++++++++++++++++++------------------
 1 files changed, 68 insertions(+), 61 deletions(-)

diff --git a/arch/powerpc/kernel/perf_event.c b/arch/powerpc/kernel/perf_event.c
index 08460a2..43b83c3 100644
--- a/arch/powerpc/kernel/perf_event.c
+++ b/arch/powerpc/kernel/perf_event.c
@@ -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,
 };
 
 /*

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-05-11 15:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-23  5:56 [RFC][PATCH v2 1/4] perf: core, add group scheduling transactional APIs Lin Ming
2010-05-07 18:44 ` [tip:perf/core] perf: Add " tip-bot for Lin Ming
2010-05-07 21:55   ` Peter Zijlstra
2010-05-08 10:28     ` Paul Mackerras
2010-05-10  8:11       ` Peter Zijlstra
2010-05-10 11:38         ` Paul Mackerras
2010-05-11 15:43       ` [tip:perf/core] perf, powerpc: Implement " tip-bot for Lin Ming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).