Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1651699 > unrolled thread
| Started by | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| First post | 2017-05-27 03:40 +0200 |
| Last post | 2017-05-30 10:30 +0200 |
| Articles | 19 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-27 03:40 +0200
[PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-27 13:30 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 09:50 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 11:20 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 12:50 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 13:00 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 13:30 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 13:50 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 09:50 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 11:30 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 12:40 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 12:50 +0200
Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi David Carrillo-Cisneros <davidcc@google.com> - 2017-05-31 23:40 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-05-29 14:10 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 15:50 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 17:30 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Peter Zijlstra <peterz@infradead.org> - 2017-05-29 17:30 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Alexey Budankov <alexey.budankov@linux.intel.com> - 2017-05-29 18:50 +0200
Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2017-05-30 10:30 +0200
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-27 03:40 +0200 |
| Subject | [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tLyRY-4YN-13@gated-at.bofh.it> |
perf/core: addressing 4x slowdown during per-process profiling of
STREAM benchmark on Intel Xeon Phi
Motivation:
The issue manifests like 4x slowdown when profiling single thread
STREAM benchmark on Intel Xeon Phi running RHEL7.2 (Intel MPSS distro).
Perf profiling is done in per-process mode and involves about 30 core
events. In case the benchmark is OpenMP based and runs under profiling
in 272 threads the overhead becomes even more dramatic:
512.144s against 36.192s (with this patch).
Solution:
cpu indexed trees for perf_event_context::pinned_group and
perf_event_context::flexible_group lists are introduced. Every tree
node keeps a list of groups allocated for the same cpu. A tree
references only groups located at the appropriate group list. The tree
provides capability to iterate over groups allocated for a specific
cpu only, what is exactly required by multiplexing timer interrupt
handler. The handler runs per-cpu and enables/disables groups using
group_sched_in()/group_sched_out() that call event_filter_match()
function filtering out groups allocated for cpus different from the
one executing the handler. Additionally for every filtered out group
group_sched_out() updates tstamps values to the current interrupt
time. This updating work is now done only once by
update_context_time() called by ctx_sched_out() before cpu groups
iteration. For this trick to work it is required that tstamps of
filtered out events would point to perf_event_context::tstamp_data
object instead of perf_event::tstam_data ones, as it is initialized
from an event allocation. tstamps references are switched by
group_sched_in()/group_sched_out() every time a group is checked for
its suitability for currently running cpu. When a thread enters some
cpu on a context switch a long run through pinned and flexible groups
is performed by perf_event_sched_in(, mux=0) with new parameter mux
set to 0 and filtered out groups tstamps are switched to
perf_event_context::tstamp_data object. Then a series of multiplexing
interrupts happens and the handler rotates the flexible groups calling
ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the
cpu tree lists only and avoiding long runs through the complete group
lists. This is where speedup comes from. Eventually when the thread
leaves the cpu ctx_sched_out(,mux=0) is called restoring tstamps
pointers to the events' perf_event::tstamp_data objects.
Locking:
The new perf_event_context::pinned_tree and
perf_event_context::flexible_tree are protected by
perf_evet_context::lock and perf_event_context::mutex, the same
locking schema as for appropriate perf_event_context::pinned_group and
perf_event_context::flexible_group. Indirect perf_event::tstamp
reference and perf_event::tstam_data object are protected by
perf_event::lock as earlier for original perf_event::tstamp_* fields.
Added API:
Objects:
1. struct perf_event_tstamp:
- enabled
- running
- stopped
2. struct perf_event:
- group_node
- group_list
- group_list_entry
- tstamp
- tstamp_data
3. struct perf_event_context:
- pinned_tree
- flexible_tree
- tstamp_data
Functions:
1. insert a group into a tree using event->cpu as a key:
int perf_cpu_tree_insert(struct rb_root *tree,
struct perf_event *event);
2. delete a group from a tree, if the group is directly attached
to a tree it also detaches all groups on the groups
group_list list:
int perf_cpu_tree_delete(struct rb_root *tree,
struct perf_event *event);
3. find group_list list by a cpu key:
struct list_head * perf_cpu_tree_find(struct rb_root *tree,
int cpu);
4. enable a pinned group on a cpu:
void ctx_sched_in_flexible_group(
struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct perf_event *group, int *can_add_hw);
5. enable a flexible group on a cpu; calls ctx_sched_in_flexible_group
and updates group->state to ERROR in case of failure:
void ctx_sched_in_pinned_group(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct perf_event *group);
6. enable per-cpu pinned tree's groups on a cpu:
void ctx_pinned_sched_in_groups(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
7. enable per-cpu flexible tree's groups on a cpu:
void ctx_flexible_sched_in_groups(
struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
8. disable per-cpu tree's groups on a cpu:
void ctx_sched_out_groups(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
9. get a group tree based on event->attr.pinned attribute value:
struct rb_root * ctx_cpu_tree(struct perf_event *event,
struct perf_event_context *ctx);
Modified API:
Objects:
1. struct perf_event
2. struct perf_event_context
Functions:
1. perf_event_alloc()
2. add_event_to_ctx()
3. perf_event_enable_on_exec()
4. __perf_install_in_context()
5. __perf_event_init_context()
6. __perf_event_mark_enabled()
7. __perf_event_enable()
8. __perf_event_task_sched_out()
9. ctx_group_list()
10. list_add_event()
11. list_del_event()
12. perf_event_context_sched_in()
13. cpu_ctx_sched_in()
14. cpu_ctx_sched_out()
15. ctx_sched_in()
16. ctx_sched_out()
17. ctx_resched()
18. ctx_pinned_sched_in()
19. ctx_flexible_sched_in()
20. group_sched_in()
21. event_sched_in()
22. event_sched_out()
23. rotate_ctx()
24. perf_rotate_context()
25. update_context_time()
26. update_event_times()
27. calc_timer_values()
28. perf_cgroup_switch()
29. perf_cgroup_mark_enabled()
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/perf_event.h | 63 +++++--
kernel/events/core.c | 442
++++++++++++++++++++++++++++++++++-----------
2 files changed, 384 insertions(+), 121 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 24a6358..6bd380f 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -550,6 +550,22 @@ struct pmu_event_list {
struct list_head list;
};
+struct perf_event_tstamp {
+ /*
+ * These are timestamps used for computing total_time_enabled
+ * and total_time_running when the event is in INACTIVE or
+ * ACTIVE state, measured in nanoseconds from an arbitrary point
+ * in time.
+ * enabled: the notional time when the event was enabled
+ * running: the notional time when the event was scheduled on
+ * stopped: in INACTIVE state, the notional time when the
+ * event was scheduled off.
+ */
+ u64 enabled;
+ u64 running;
+ u64 stopped;
+};
+
/**
* struct perf_event - performance event kernel representation:
*/
@@ -571,6 +587,27 @@ struct perf_event {
* either sufficies for read.
*/
struct list_head group_entry;
+ /*
+ * Node on the pinned or flexible tree located at the event context;
+ * the node may be empty in case its event is not directly attached
+ * to the tree but to group_list list of the event directly
+ * attached to the tree;
+ */
+ struct rb_node group_node;
+ /*
+ * List keeps groups allocated for the same cpu;
+ * the list may be empty in case its event is not directly
+ * attached to the tree but to group_list list of the event directly
+ * attached to the tree;
+ */
+ struct list_head group_list;
+ /*
+ * Entry into the group_list list above;
+ * the entry may be attached to the self group_list list above
+ * in case the event is directly attached to the pinned or
+ * flexible tree;
+ */
+ struct list_head group_list_entry;
struct list_head sibling_list;
/*
@@ -611,18 +648,11 @@ struct perf_event {
u64 total_time_running;
/*
- * These are timestamps used for computing total_time_enabled
- * and total_time_running when the event is in INACTIVE or
- * ACTIVE state, measured in nanoseconds from an arbitrary point
- * in time.
- * tstamp_enabled: the notional time when the event was enabled
- * tstamp_running: the notional time when the event was scheduled on
- * tstamp_stopped: in INACTIVE state, the notional time when the
- * event was scheduled off.
+ * tstamp points to the tstamp_data object below or to the object
+ * located at the event context;
*/
- u64 tstamp_enabled;
- u64 tstamp_running;
- u64 tstamp_stopped;
+ struct perf_event_tstamp *tstamp;
+ struct perf_event_tstamp tstamp_data;
/*
* timestamp shadows the actual context timing but it can
@@ -742,7 +772,17 @@ struct perf_event_context {
struct list_head active_ctx_list;
struct list_head pinned_groups;
+ /*
+ * Cpu tree for pinned groups; keeps event's group_node nodes
+ * of attached flexible groups;
+ */
+ struct rb_root pinned_tree;
struct list_head flexible_groups;
+ /*
+ * Cpu tree for flexible groups; keeps event's group_node nodes
+ * of attached flexible groups;
+ */
+ struct rb_root flexible_tree;
struct list_head event_list;
int nr_events;
int nr_active;
@@ -758,6 +798,7 @@ struct perf_event_context {
*/
u64 time;
u64 timestamp;
+ struct perf_event_tstamp tstamp_data;
/*
* These fields let us detect when two contexts have both
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f8c27d3..9168406 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -555,11 +555,11 @@ void perf_sample_event_took(u64 sample_len_ns)
static atomic64_t perf_event_id;
static void cpu_ctx_sched_out(struct perf_cpu_context *cpuctx,
- enum event_type_t event_type);
+ enum event_type_t event_type, int mux);
static void cpu_ctx_sched_in(struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task);
+ struct task_struct *task, int mux);
static void update_context_time(struct perf_event_context *ctx);
static u64 perf_event_time(struct perf_event *event);
@@ -701,6 +701,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
struct perf_cpu_context *cpuctx;
struct list_head *list;
unsigned long flags;
+ int mux = 0;
/*
* Disable interrupts and preemption to avoid this CPU's
@@ -716,7 +717,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
perf_pmu_disable(cpuctx->ctx.pmu);
if (mode & PERF_CGROUP_SWOUT) {
- cpu_ctx_sched_out(cpuctx, EVENT_ALL);
+ cpu_ctx_sched_out(cpuctx, EVENT_ALL, mux);
/*
* must not be done before ctxswout due
* to event_filter_match() in event_sched_out()
@@ -735,7 +736,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
*/
cpuctx->cgrp = perf_cgroup_from_task(task,
&cpuctx->ctx);
- cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
+ cpu_ctx_sched_in(cpuctx, EVENT_ALL, task, mux);
}
perf_pmu_enable(cpuctx->ctx.pmu);
perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
@@ -865,10 +866,10 @@ perf_cgroup_mark_enabled(struct perf_event *event,
event->cgrp_defer_enabled = 0;
- event->tstamp_enabled = tstamp - event->total_time_enabled;
+ event->tstamp->enabled = tstamp - event->total_time_enabled;
list_for_each_entry(sub, &event->sibling_list, group_entry) {
if (sub->state >= PERF_EVENT_STATE_INACTIVE) {
- sub->tstamp_enabled = tstamp - sub->total_time_enabled;
+ sub->tstamp->enabled = tstamp - sub->total_time_enabled;
sub->cgrp_defer_enabled = 0;
}
}
@@ -1383,6 +1384,9 @@ static void update_context_time(struct
perf_event_context *ctx)
ctx->time += now - ctx->timestamp;
ctx->timestamp = now;
+
+ ctx->tstamp_data.running += ctx->time - ctx->tstamp_data.stopped;
+ ctx->tstamp_data.stopped = ctx->time;
}
static u64 perf_event_time(struct perf_event *event)
@@ -1424,16 +1428,16 @@ static void update_event_times(struct perf_event
*event)
else if (ctx->is_active)
run_end = ctx->time;
else
- run_end = event->tstamp_stopped;
+ run_end = event->tstamp->stopped;
- event->total_time_enabled = run_end - event->tstamp_enabled;
+ event->total_time_enabled = run_end - event->tstamp->enabled;
if (event->state == PERF_EVENT_STATE_INACTIVE)
- run_end = event->tstamp_stopped;
+ run_end = event->tstamp->stopped;
else
run_end = perf_event_time(event);
- event->total_time_running = run_end - event->tstamp_running;
+ event->total_time_running = run_end - event->tstamp->running;
}
@@ -1472,6 +1476,186 @@ ctx_group_list(struct perf_event *event, struct
perf_event_context *ctx)
return &ctx->flexible_groups;
}
+static int
+perf_cpu_tree_insert(struct rb_root *tree, struct perf_event *event)
+{
+ struct rb_node **node;
+ struct rb_node *parent;
+
+ if (!tree || !event)
+ return 0;
+
+ node = &tree->rb_node;
+ parent = *node;
+
+ while (*node) {
+ struct perf_event *node_event = container_of(*node,
+ struct perf_event, group_node);
+
+ parent = *node;
+
+ if (event->cpu < node_event->cpu) {
+ node = &((*node)->rb_left);
+ } else if (event->cpu > node_event->cpu) {
+ node = &((*node)->rb_right);
+ } else {
+ list_add_tail(&event->group_list_entry,
+ &node_event->group_list);
+ return 2;
+ }
+ }
+
+ list_add_tail(&event->group_list_entry, &event->group_list);
+
+ rb_link_node(&event->group_node, parent, node);
+ rb_insert_color(&event->group_node, tree);
+
+ return 1;
+}
+
+static int
+perf_cpu_tree_delete(struct rb_root *tree, struct perf_event *event)
+{
+ if (!tree || !event)
+ return 0;
+
+ if (RB_EMPTY_NODE(&event->group_node)) {
+ list_del_init(&event->group_list_entry);
+ } else {
+ struct perf_event *list_event, *list_next;
+
+ rb_erase(&event->group_node, tree);
+ list_for_each_entry_safe(list_event, list_next,
+ &event->group_list, group_list_entry)
+ list_del_init(&list_event->group_list_entry);
+ }
+
+ return 1;
+}
+
+static struct list_head *
+perf_cpu_tree_find(struct rb_root *tree, int cpu)
+{
+ struct rb_node *node;
+
+ if (!tree)
+ return NULL;
+
+ node = tree->rb_node;
+
+ while (node) {
+ struct perf_event *node_event = container_of(node,
+ struct perf_event, group_node);
+
+ if (cpu < node_event->cpu)
+ node = node->rb_left;
+ else if (cpu > node_event->cpu)
+ node = node->rb_right;
+ else
+ return &node_event->group_list;
+ }
+
+ return NULL;
+}
+
+static struct rb_root *
+ctx_cpu_tree(struct perf_event *event, struct perf_event_context *ctx)
+{
+ if (event->attr.pinned)
+ return &ctx->pinned_tree;
+ else
+ return &ctx->flexible_tree;
+}
+
+static inline int
+event_filter_match(struct perf_event *event);
+
+static int
+group_can_go_on(struct perf_event *event,
+ struct perf_cpu_context *cpuctx,
+ int can_add_hw);
+
+static int
+group_sched_in(struct perf_event *group_event,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx);
+
+static void
+ctx_sched_in_flexible_group(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event *event, int *can_add_hw)
+{
+ if (event->state <= PERF_EVENT_STATE_OFF)
+ return;
+ if (!event_filter_match(event)) {
+ if (event->tstamp != &ctx->tstamp_data)
+ event->tstamp = &ctx->tstamp_data;
+ return;
+ }
+
+ /* may need to reset tstamp_enabled */
+ if (is_cgroup_event(event))
+ perf_cgroup_mark_enabled(event, ctx);
+
+ if (group_can_go_on(event, cpuctx, *can_add_hw)) {
+ if (group_sched_in(event, cpuctx, ctx))
+ *can_add_hw = 0;
+ }
+}
+
+static void
+ctx_sched_in_pinned_group(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event *event)
+{
+ int can_add_hw = 1;
+
+ ctx_sched_in_flexible_group(ctx, cpuctx, event, &can_add_hw);
+ if (!can_add_hw) {
+ update_group_times(event);
+ event->state = PERF_EVENT_STATE_ERROR;
+ }
+}
+
+static void
+ctx_pinned_sched_in_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ ctx_sched_in_pinned_group(ctx, cpuctx, group);
+}
+
+static void
+ctx_flexible_sched_in_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+ int can_add_hw = 1;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ ctx_sched_in_flexible_group(ctx, cpuctx, group, &can_add_hw);
+}
+
+static void
+group_sched_out(struct perf_event *group_event,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx);
+
+static void
+ctx_sched_out_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ group_sched_out(group, cpuctx, ctx);
+}
+
/*
* Add a event from the lists for its context.
* Must be called with ctx->mutex and ctx->lock held.
@@ -1491,11 +1675,15 @@ list_add_event(struct perf_event *event, struct
perf_event_context *ctx)
*/
if (event->group_leader == event) {
struct list_head *list;
+ struct rb_root *tree;
event->group_caps = event->event_caps;
list = ctx_group_list(event, ctx);
list_add_tail(&event->group_entry, list);
+
+ tree = ctx_cpu_tree(event, ctx);
+ perf_cpu_tree_insert(tree, event);
}
list_update_cgroup_event(event, ctx, true);
@@ -1685,8 +1873,14 @@ list_del_event(struct perf_event *event, struct
perf_event_context *ctx)
list_del_rcu(&event->event_entry);
- if (event->group_leader == event)
+ if (event->group_leader == event) {
+ struct rb_root *tree;
+
+ tree = ctx_cpu_tree(event, ctx);
+ perf_cpu_tree_delete(tree, event);
+
list_del_init(&event->group_entry);
+ }
update_group_times(event);
@@ -1811,9 +2005,9 @@ event_sched_out(struct perf_event *event,
*/
if (event->state == PERF_EVENT_STATE_INACTIVE &&
!event_filter_match(event)) {
- delta = tstamp - event->tstamp_stopped;
- event->tstamp_running += delta;
- event->tstamp_stopped = tstamp;
+ delta = tstamp - event->tstamp->stopped;
+ event->tstamp->running += delta;
+ event->tstamp->stopped = tstamp;
}
if (event->state != PERF_EVENT_STATE_ACTIVE)
@@ -1821,7 +2015,7 @@ event_sched_out(struct perf_event *event,
perf_pmu_disable(event->pmu);
- event->tstamp_stopped = tstamp;
+ event->tstamp->stopped = tstamp;
event->pmu->del(event, 0);
event->oncpu = -1;
event->state = PERF_EVENT_STATE_INACTIVE;
@@ -2096,7 +2290,7 @@ event_sched_in(struct perf_event *event,
goto out;
}
- event->tstamp_running += tstamp - event->tstamp_stopped;
+ event->tstamp->running += tstamp - event->tstamp->stopped;
if (!is_software_event(event))
cpuctx->active_oncpu++;
@@ -2168,8 +2362,8 @@ group_sched_in(struct perf_event *group_event,
simulate = true;
if (simulate) {
- event->tstamp_running += now - event->tstamp_stopped;
- event->tstamp_stopped = now;
+ event->tstamp->running += now - event->tstamp->stopped;
+ event->tstamp->stopped = now;
} else {
event_sched_out(event, cpuctx, ctx);
}
@@ -2221,43 +2415,45 @@ static void add_event_to_ctx(struct perf_event
*event,
list_add_event(event, ctx);
perf_group_attach(event);
- event->tstamp_enabled = tstamp;
- event->tstamp_running = tstamp;
- event->tstamp_stopped = tstamp;
+ event->tstamp->enabled = tstamp;
+ event->tstamp->running = tstamp;
+ event->tstamp->stopped = tstamp;
}
static void ctx_sched_out(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
- enum event_type_t event_type);
+ enum event_type_t event_type, int mux);
static void
ctx_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task);
+ struct task_struct *task, int mux);
static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
struct perf_event_context *ctx,
enum event_type_t event_type)
{
+ int mux = 0;
+
if (!cpuctx->task_ctx)
return;
if (WARN_ON_ONCE(ctx != cpuctx->task_ctx))
return;
- ctx_sched_out(ctx, cpuctx, event_type);
+ ctx_sched_out(ctx, cpuctx, event_type, mux);
}
static void perf_event_sched_in(struct perf_cpu_context *cpuctx,
struct perf_event_context *ctx,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
- cpu_ctx_sched_in(cpuctx, EVENT_PINNED, task);
+ cpu_ctx_sched_in(cpuctx, EVENT_PINNED, task, mux);
if (ctx)
- ctx_sched_in(ctx, cpuctx, EVENT_PINNED, task);
- cpu_ctx_sched_in(cpuctx, EVENT_FLEXIBLE, task);
+ ctx_sched_in(ctx, cpuctx, EVENT_PINNED, task, mux);
+ cpu_ctx_sched_in(cpuctx, EVENT_FLEXIBLE, task, mux);
if (ctx)
- ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task);
+ ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task, mux);
}
/*
@@ -2281,6 +2477,7 @@ static void ctx_resched(struct perf_cpu_context
*cpuctx,
{
enum event_type_t ctx_event_type = event_type & EVENT_ALL;
bool cpu_event = !!(event_type & EVENT_CPU);
+ int mux = 0;
/*
* If pinned groups are involved, flexible groups also need to be
@@ -2301,11 +2498,11 @@ static void ctx_resched(struct perf_cpu_context
*cpuctx,
* - otherwise, do nothing more.
*/
if (cpu_event)
- cpu_ctx_sched_out(cpuctx, ctx_event_type);
+ cpu_ctx_sched_out(cpuctx, ctx_event_type, mux);
else if (ctx_event_type & EVENT_PINNED)
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
- perf_event_sched_in(cpuctx, task_ctx, current);
+ perf_event_sched_in(cpuctx, task_ctx, current, mux);
perf_pmu_enable(cpuctx->ctx.pmu);
}
@@ -2323,6 +2520,7 @@ static int __perf_install_in_context(void *info)
struct perf_event_context *task_ctx = cpuctx->task_ctx;
bool reprogram = true;
int ret = 0;
+ int mux = 0;
raw_spin_lock(&cpuctx->ctx.lock);
if (ctx->task) {
@@ -2349,7 +2547,7 @@ static int __perf_install_in_context(void *info)
}
if (reprogram) {
- ctx_sched_out(ctx, cpuctx, EVENT_TIME);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
add_event_to_ctx(event, ctx);
ctx_resched(cpuctx, task_ctx, get_event_type(event));
} else {
@@ -2468,10 +2666,10 @@ static void __perf_event_mark_enabled(struct
perf_event *event)
u64 tstamp = perf_event_time(event);
event->state = PERF_EVENT_STATE_INACTIVE;
- event->tstamp_enabled = tstamp - event->total_time_enabled;
+ event->tstamp->enabled = tstamp - event->total_time_enabled;
list_for_each_entry(sub, &event->sibling_list, group_entry) {
if (sub->state >= PERF_EVENT_STATE_INACTIVE)
- sub->tstamp_enabled = tstamp - sub->total_time_enabled;
+ sub->tstamp->enabled = tstamp - sub->total_time_enabled;
}
}
@@ -2485,13 +2683,14 @@ static void __perf_event_enable(struct
perf_event *event,
{
struct perf_event *leader = event->group_leader;
struct perf_event_context *task_ctx;
+ int mux = 0;
if (event->state >= PERF_EVENT_STATE_INACTIVE ||
event->state <= PERF_EVENT_STATE_ERROR)
return;
if (ctx->is_active)
- ctx_sched_out(ctx, cpuctx, EVENT_TIME);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
__perf_event_mark_enabled(event);
@@ -2501,7 +2700,7 @@ static void __perf_event_enable(struct perf_event
*event,
if (!event_filter_match(event)) {
if (is_cgroup_event(event))
perf_cgroup_defer_enabled(event);
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
return;
}
@@ -2510,7 +2709,7 @@ static void __perf_event_enable(struct perf_event
*event,
* then don't put it on unless the group is on.
*/
if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE) {
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
return;
}
@@ -2706,10 +2905,12 @@ EXPORT_SYMBOL_GPL(perf_event_refresh);
static void ctx_sched_out(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
- enum event_type_t event_type)
+ enum event_type_t event_type, int mux)
{
int is_active = ctx->is_active;
struct perf_event *event;
+ struct list_head *groups;
+ int cpu = smp_processor_id();
lockdep_assert_held(&ctx->lock);
@@ -2756,13 +2957,34 @@ static void ctx_sched_out(struct
perf_event_context *ctx,
perf_pmu_disable(ctx->pmu);
if (is_active & EVENT_PINNED) {
- list_for_each_entry(event, &ctx->pinned_groups, group_entry)
- group_sched_out(event, cpuctx, ctx);
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, -1);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, cpu);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ } else {
+ list_for_each_entry(event, &ctx->pinned_groups,
+ group_entry)
+ group_sched_out(event, cpuctx, ctx);
+ }
}
if (is_active & EVENT_FLEXIBLE) {
- list_for_each_entry(event, &ctx->flexible_groups, group_entry)
- group_sched_out(event, cpuctx, ctx);
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, cpu);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+
+ } else {
+ list_for_each_entry(event, &ctx->flexible_groups,
+ group_entry)
+ group_sched_out(event, cpuctx, ctx);
+ }
}
perf_pmu_enable(ctx->pmu);
}
@@ -3051,9 +3273,9 @@ void __perf_event_task_sched_out(struct
task_struct *task,
* Called with IRQs disabled
*/
static void cpu_ctx_sched_out(struct perf_cpu_context *cpuctx,
- enum event_type_t event_type)
+ enum event_type_t event_type, int mux)
{
- ctx_sched_out(&cpuctx->ctx, cpuctx, event_type);
+ ctx_sched_out(&cpuctx->ctx, cpuctx, event_type, mux);
}
static void
@@ -3061,29 +3283,8 @@ ctx_pinned_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx)
{
struct perf_event *event;
-
- list_for_each_entry(event, &ctx->pinned_groups, group_entry) {
- if (event->state <= PERF_EVENT_STATE_OFF)
- continue;
- if (!event_filter_match(event))
- continue;
-
- /* may need to reset tstamp_enabled */
- if (is_cgroup_event(event))
- perf_cgroup_mark_enabled(event, ctx);
-
- if (group_can_go_on(event, cpuctx, 1))
- group_sched_in(event, cpuctx, ctx);
-
- /*
- * If this pinned group hasn't been scheduled,
- * put it in error state.
- */
- if (event->state == PERF_EVENT_STATE_INACTIVE) {
- update_group_times(event);
- event->state = PERF_EVENT_STATE_ERROR;
- }
- }
+ list_for_each_entry(event, &ctx->pinned_groups, group_entry)
+ ctx_sched_in_pinned_group(ctx, cpuctx, event);
}
static void
@@ -3092,37 +3293,19 @@ ctx_flexible_sched_in(struct perf_event_context
*ctx,
{
struct perf_event *event;
int can_add_hw = 1;
-
- list_for_each_entry(event, &ctx->flexible_groups, group_entry) {
- /* Ignore events in OFF or ERROR state */
- if (event->state <= PERF_EVENT_STATE_OFF)
- continue;
- /*
- * Listen to the 'cpu' scheduling filter constraint
- * of events:
- */
- if (!event_filter_match(event))
- continue;
-
- /* may need to reset tstamp_enabled */
- if (is_cgroup_event(event))
- perf_cgroup_mark_enabled(event, ctx);
-
- if (group_can_go_on(event, cpuctx, can_add_hw)) {
- if (group_sched_in(event, cpuctx, ctx))
- can_add_hw = 0;
- }
- }
+ list_for_each_entry(event, &ctx->flexible_groups, group_entry)
+ ctx_sched_in_flexible_group(ctx, cpuctx, event, &can_add_hw);
}
static void
ctx_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
int is_active = ctx->is_active;
- u64 now;
+ struct list_head *groups;
+ int cpu = smp_processor_id();
lockdep_assert_held(&ctx->lock);
@@ -3141,8 +3324,7 @@ ctx_sched_in(struct perf_event_context *ctx,
if (is_active & EVENT_TIME) {
/* start ctx time */
- now = perf_clock();
- ctx->timestamp = now;
+ ctx->timestamp = perf_clock();
perf_cgroup_set_timestamp(task, ctx);
}
@@ -3150,27 +3332,50 @@ ctx_sched_in(struct perf_event_context *ctx,
* First go through the list and put on any pinned groups
* in order to give them the best chance of going on.
*/
- if (is_active & EVENT_PINNED)
- ctx_pinned_sched_in(ctx, cpuctx);
+ if (is_active & EVENT_PINNED) {
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, -1);
+ if (groups)
+ ctx_pinned_sched_in_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, cpu);
+ if (groups)
+ ctx_pinned_sched_in_groups(ctx, cpuctx, groups);
+ } else {
+ ctx_pinned_sched_in(ctx, cpuctx);
+ }
+ }
/* Then walk through the lower prio flexible groups */
- if (is_active & EVENT_FLEXIBLE)
- ctx_flexible_sched_in(ctx, cpuctx);
+ if (is_active & EVENT_FLEXIBLE) {
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ ctx_flexible_sched_in_groups(ctx, cpuctx,
+ groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, cpu);
+ if (groups)
+ ctx_flexible_sched_in_groups(ctx, cpuctx,
+ groups);
+ } else {
+ ctx_flexible_sched_in(ctx, cpuctx);
+ }
+ }
}
static void cpu_ctx_sched_in(struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
struct perf_event_context *ctx = &cpuctx->ctx;
- ctx_sched_in(ctx, cpuctx, event_type, task);
+ ctx_sched_in(ctx, cpuctx, event_type, task, mux);
}
static void perf_event_context_sched_in(struct perf_event_context *ctx,
struct task_struct *task)
{
struct perf_cpu_context *cpuctx;
+ int mux = 0;
cpuctx = __get_cpu_context(ctx);
if (cpuctx->task_ctx == ctx)
@@ -3187,8 +3392,8 @@ static void perf_event_context_sched_in(struct
perf_event_context *ctx,
* events, no need to flip the cpuctx's events around.
*/
if (!list_empty(&ctx->pinned_groups))
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
- perf_event_sched_in(cpuctx, ctx, task);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
+ perf_event_sched_in(cpuctx, ctx, task, mux);
perf_pmu_enable(ctx->pmu);
perf_ctx_unlock(cpuctx, ctx);
}
@@ -3421,14 +3626,24 @@ static void rotate_ctx(struct perf_event_context
*ctx)
* Rotate the first entry last of non-pinned groups. Rotation might be
* disabled by the inheritance code.
*/
- if (!ctx->rotate_disable)
- list_rotate_left(&ctx->flexible_groups);
+ if (!ctx->rotate_disable) {
+ struct list_head *groups;
+
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ list_rotate_left(groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree,
+ smp_processor_id());
+ if (groups)
+ list_rotate_left(groups);
+ }
}
static int perf_rotate_context(struct perf_cpu_context *cpuctx)
{
struct perf_event_context *ctx = NULL;
int rotate = 0;
+ int mux = 1;
if (cpuctx->ctx.nr_events) {
if (cpuctx->ctx.nr_events != cpuctx->ctx.nr_active)
@@ -3447,15 +3662,15 @@ static int perf_rotate_context(struct
perf_cpu_context *cpuctx)
perf_ctx_lock(cpuctx, cpuctx->task_ctx);
perf_pmu_disable(cpuctx->ctx.pmu);
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
if (ctx)
- ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE);
+ ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE, mux);
rotate_ctx(&cpuctx->ctx);
if (ctx)
rotate_ctx(ctx);
- perf_event_sched_in(cpuctx, ctx, current);
+ perf_event_sched_in(cpuctx, ctx, current, mux);
perf_pmu_enable(cpuctx->ctx.pmu);
perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
@@ -3507,6 +3722,7 @@ static void perf_event_enable_on_exec(int ctxn)
struct perf_event *event;
unsigned long flags;
int enabled = 0;
+ int mux = 0;
local_irq_save(flags);
ctx = current->perf_event_ctxp[ctxn];
@@ -3515,7 +3731,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);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
list_for_each_entry(event, &ctx->event_list, event_entry) {
enabled |= event_enable_on_exec(event, ctx);
event_type |= get_event_type(event);
@@ -3528,7 +3744,7 @@ static void perf_event_enable_on_exec(int ctxn)
clone_ctx = unclone_ctx(ctx);
ctx_resched(cpuctx, ctx, event_type);
} else {
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
}
perf_ctx_unlock(cpuctx, ctx);
@@ -3749,7 +3965,9 @@ static void __perf_event_init_context(struct
perf_event_context *ctx)
mutex_init(&ctx->mutex);
INIT_LIST_HEAD(&ctx->active_ctx_list);
INIT_LIST_HEAD(&ctx->pinned_groups);
+ ctx->pinned_tree = RB_ROOT;
INIT_LIST_HEAD(&ctx->flexible_groups);
+ ctx->flexible_tree = RB_ROOT;
INIT_LIST_HEAD(&ctx->event_list);
atomic_set(&ctx->refcount, 1);
}
@@ -4848,8 +5066,8 @@ static void calc_timer_values(struct perf_event
*event,
*now = perf_clock();
ctx_time = event->shadow_ctx_time + *now;
- *enabled = ctx_time - event->tstamp_enabled;
- *running = ctx_time - event->tstamp_running;
+ *enabled = ctx_time - event->tstamp->enabled;
+ *running = ctx_time - event->tstamp->running;
}
static void perf_event_init_userpage(struct perf_event *event)
@@ -9357,6 +9575,9 @@ perf_event_alloc(struct perf_event_attr *attr, int
cpu,
INIT_LIST_HEAD(&event->child_list);
INIT_LIST_HEAD(&event->group_entry);
+ RB_CLEAR_NODE(&event->group_node);
+ INIT_LIST_HEAD(&event->group_list);
+ INIT_LIST_HEAD(&event->group_list_entry);
INIT_LIST_HEAD(&event->event_entry);
INIT_LIST_HEAD(&event->sibling_list);
INIT_LIST_HEAD(&event->rb_entry);
@@ -9372,6 +9593,7 @@ perf_event_alloc(struct perf_event_attr *attr, int
cpu,
raw_spin_lock_init(&event->addr_filters.lock);
atomic_long_set(&event->refcount, 1);
+ event->tstamp = &event->tstamp_data;
event->cpu = cpu;
event->attr = *attr;
event->group_leader = group_leader;
[toc] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-27 13:30 +0200 |
| Subject | [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tLI4V-2JT-1@gated-at.bofh.it> |
| In reply to | #1651699 |
Motivation:
The issue manifests like 4x slowdown when profiling single thread STREAM
benchmark on Intel Xeon Phi running RHEL7.2 (Intel MPSS distribution).
Perf profiling is done in per-process mode and involves about 30 core
events. In case the benchmark is OpenMP based and runs under profiling in
272 threads the overhead becomes even more dramatic: 512.144s against
36.192s (with this patch).
Solution:
cpu indexed trees for perf_event_context::pinned_groups and
perf_event_context::flexible_groups lists are introduced. Every tree node
keeps a list of groups allocated for the same cpu. A tree references only
groups located at the appropriate group list. The tree provides capability
to iterate over groups allocated for a specific cpu only, what is exactly
required by multiplexing timer interrupt handler. The handler runs per-cpu
and enables/disables groups using group_sched_in()/group_sched_out() that
call event_filter_match() function filtering out groups allocated for cpus
different from the one executing the handler. Additionally for every
filtered out group group_sched_out() updates tstamps values to the current
interrupt time. This updating work is now done only once by
update_context_time() called by ctx_sched_out() before cpu groups
iteration. For this trick to work it is required that tstamps of filtered
out events would point to perf_event_context::tstamp_data object instead
of perf_event::tstamp_data ones, as it is initialized from an event
allocation. tstamps references are switched by
group_sched_in()/group_sched_out() every time a group is checked for its
suitability for currently running cpu. When a thread enters some cpu on
a context switch a long run through pinned and flexible groups is
performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0
and filtered out groups tstamps are switched to
perf_event_context::tstamp_data object. Then a series of multiplexing
interrupts happens and the handler rotates the flexible groups calling
ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu
tree lists only and avoiding long runs through the complete group lists.
This is where speedup comes from. Eventually when the thread leaves the cpu
ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events'
perf_event::tstamp_data objects.
Locking:
The new perf_event_context::pinned_tree and
perf_event_context::flexible_tree are protected by perf_evet_context::lock
and perf_event_context::mutex, the same locking schema as for appropriate
perf_event_context::pinned_group and perf_event_context::flexible_group.
Indirect perf_event::tstamp reference and perf_event::tstamp_data object
are protected by perf_event::lock as earlier for original
perf_event::tstamp_* fields.
Added API:
Objects:
1. struct perf_event_tstamp:
- enabled
- running
- stopped
2. struct perf_event:
- group_node
- group_list
- group_list_entry
- tstamp
- tstamp_data
3. struct perf_event_context:
- pinned_tree
- flexible_tree
- tstamp_data
Functions:
1. insert a group into a tree using event->cpu as a key:
int perf_cpu_tree_insert(struct rb_root *tree,
struct perf_event *event);
2. delete a group from a tree, if the group is directly attached
to a tree it also detaches all groups on the groups
group_list list:
int perf_cpu_tree_delete(struct rb_root *tree,
struct perf_event *event);
3. find group_list list by a cpu key:
struct list_head * perf_cpu_tree_find(struct rb_root *tree,
int cpu);
4. enable a pinned group on a cpu:
void ctx_sched_in_flexible_group(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct perf_event *group, int *can_add_hw);
5. enable a flexible group on a cpu; calls ctx_sched_in_flexible_group
and updates group->state to ERROR in case of failure:
void ctx_sched_in_pinned_group(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct perf_event *group);
6. enable per-cpu pinned tree's groups on a cpu:
void ctx_pinned_sched_in_groups(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
7. enable per-cpu flexible tree's groups on a cpu:
void ctx_flexible_sched_in_groups(
struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
8. disable per-cpu tree's groups on a cpu:
void ctx_sched_out_groups(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
struct list_head *groups);
9. get a group tree based on event->attr.pinned attribute value:
struct rb_root * ctx_cpu_tree(struct perf_event *event,
struct perf_event_context *ctx);
Modified API:
Objects:
1. struct perf_event
2. struct perf_event_context
Functions:
1. perf_event_alloc()
2. add_event_to_ctx()
3. perf_event_enable_on_exec()
4. __perf_install_in_context()
5. __perf_event_init_context()
6. __perf_event_mark_enabled()
7. __perf_event_enable()
8. __perf_event_task_sched_out()
9. ctx_group_list()
10. list_add_event()
11. list_del_event()
12. perf_event_context_sched_in()
13. cpu_ctx_sched_in()
14. cpu_ctx_sched_out()
15. ctx_sched_in()
16. ctx_sched_out()
17. ctx_resched()
18. ctx_pinned_sched_in()
19. ctx_flexible_sched_in()
20. group_sched_in()
21. event_sched_in()
22. event_sched_out()
23. rotate_ctx()
24. perf_rotate_context()
25. update_context_time()
26. update_event_times()
27. calc_timer_values()
28. perf_cgroup_switch()
29. perf_cgroup_mark_enabled()
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
include/linux/perf_event.h | 63 +++++--
kernel/events/core.c | 446
++++++++++++++++++++++++++++++++++-----------
2 files changed, 388 insertions(+), 121 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 24a6358..6bd380f 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -550,6 +550,22 @@ struct pmu_event_list {
struct list_head list;
};
+struct perf_event_tstamp {
+ /*
+ * These are timestamps used for computing total_time_enabled
+ * and total_time_running when the event is in INACTIVE or
+ * ACTIVE state, measured in nanoseconds from an arbitrary point
+ * in time.
+ * enabled: the notional time when the event was enabled
+ * running: the notional time when the event was scheduled on
+ * stopped: in INACTIVE state, the notional time when the
+ * event was scheduled off.
+ */
+ u64 enabled;
+ u64 running;
+ u64 stopped;
+};
+
/**
* struct perf_event - performance event kernel representation:
*/
@@ -571,6 +587,27 @@ struct perf_event {
* either sufficies for read.
*/
struct list_head group_entry;
+ /*
+ * Node on the pinned or flexible tree located at the event context;
+ * the node may be empty in case its event is not directly attached
+ * to the tree but to group_list list of the event directly
+ * attached to the tree;
+ */
+ struct rb_node group_node;
+ /*
+ * List keeps groups allocated for the same cpu;
+ * the list may be empty in case its event is not directly
+ * attached to the tree but to group_list list of the event directly
+ * attached to the tree;
+ */
+ struct list_head group_list;
+ /*
+ * Entry into the group_list list above;
+ * the entry may be attached to the self group_list list above
+ * in case the event is directly attached to the pinned or
+ * flexible tree;
+ */
+ struct list_head group_list_entry;
struct list_head sibling_list;
/*
@@ -611,18 +648,11 @@ struct perf_event {
u64 total_time_running;
/*
- * These are timestamps used for computing total_time_enabled
- * and total_time_running when the event is in INACTIVE or
- * ACTIVE state, measured in nanoseconds from an arbitrary point
- * in time.
- * tstamp_enabled: the notional time when the event was enabled
- * tstamp_running: the notional time when the event was scheduled on
- * tstamp_stopped: in INACTIVE state, the notional time when the
- * event was scheduled off.
+ * tstamp points to the tstamp_data object below or to the object
+ * located at the event context;
*/
- u64 tstamp_enabled;
- u64 tstamp_running;
- u64 tstamp_stopped;
+ struct perf_event_tstamp *tstamp;
+ struct perf_event_tstamp tstamp_data;
/*
* timestamp shadows the actual context timing but it can
@@ -742,7 +772,17 @@ struct perf_event_context {
struct list_head active_ctx_list;
struct list_head pinned_groups;
+ /*
+ * Cpu tree for pinned groups; keeps event's group_node nodes
+ * of attached flexible groups;
+ */
+ struct rb_root pinned_tree;
struct list_head flexible_groups;
+ /*
+ * Cpu tree for flexible groups; keeps event's group_node nodes
+ * of attached flexible groups;
+ */
+ struct rb_root flexible_tree;
struct list_head event_list;
int nr_events;
int nr_active;
@@ -758,6 +798,7 @@ struct perf_event_context {
*/
u64 time;
u64 timestamp;
+ struct perf_event_tstamp tstamp_data;
/*
* These fields let us detect when two contexts have both
diff --git a/kernel/events/core.c b/kernel/events/core.c
index f8c27d3..92f797a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -555,11 +555,11 @@ void perf_sample_event_took(u64 sample_len_ns)
static atomic64_t perf_event_id;
static void cpu_ctx_sched_out(struct perf_cpu_context *cpuctx,
- enum event_type_t event_type);
+ enum event_type_t event_type, int mux);
static void cpu_ctx_sched_in(struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task);
+ struct task_struct *task, int mux);
static void update_context_time(struct perf_event_context *ctx);
static u64 perf_event_time(struct perf_event *event);
@@ -701,6 +701,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
struct perf_cpu_context *cpuctx;
struct list_head *list;
unsigned long flags;
+ int mux = 0;
/*
* Disable interrupts and preemption to avoid this CPU's
@@ -716,7 +717,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
perf_pmu_disable(cpuctx->ctx.pmu);
if (mode & PERF_CGROUP_SWOUT) {
- cpu_ctx_sched_out(cpuctx, EVENT_ALL);
+ cpu_ctx_sched_out(cpuctx, EVENT_ALL, mux);
/*
* must not be done before ctxswout due
* to event_filter_match() in event_sched_out()
@@ -735,7 +736,7 @@ static void perf_cgroup_switch(struct task_struct
*task, int mode)
*/
cpuctx->cgrp = perf_cgroup_from_task(task,
&cpuctx->ctx);
- cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
+ cpu_ctx_sched_in(cpuctx, EVENT_ALL, task, mux);
}
perf_pmu_enable(cpuctx->ctx.pmu);
perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
@@ -865,10 +866,10 @@ perf_cgroup_mark_enabled(struct perf_event *event,
event->cgrp_defer_enabled = 0;
- event->tstamp_enabled = tstamp - event->total_time_enabled;
+ event->tstamp->enabled = tstamp - event->total_time_enabled;
list_for_each_entry(sub, &event->sibling_list, group_entry) {
if (sub->state >= PERF_EVENT_STATE_INACTIVE) {
- sub->tstamp_enabled = tstamp - sub->total_time_enabled;
+ sub->tstamp->enabled = tstamp - sub->total_time_enabled;
sub->cgrp_defer_enabled = 0;
}
}
@@ -1383,6 +1384,9 @@ static void update_context_time(struct
perf_event_context *ctx)
ctx->time += now - ctx->timestamp;
ctx->timestamp = now;
+
+ ctx->tstamp_data.running += ctx->time - ctx->tstamp_data.stopped;
+ ctx->tstamp_data.stopped = ctx->time;
}
static u64 perf_event_time(struct perf_event *event)
@@ -1424,16 +1428,16 @@ static void update_event_times(struct perf_event
*event)
else if (ctx->is_active)
run_end = ctx->time;
else
- run_end = event->tstamp_stopped;
+ run_end = event->tstamp->stopped;
- event->total_time_enabled = run_end - event->tstamp_enabled;
+ event->total_time_enabled = run_end - event->tstamp->enabled;
if (event->state == PERF_EVENT_STATE_INACTIVE)
- run_end = event->tstamp_stopped;
+ run_end = event->tstamp->stopped;
else
run_end = perf_event_time(event);
- event->total_time_running = run_end - event->tstamp_running;
+ event->total_time_running = run_end - event->tstamp->running;
}
@@ -1472,6 +1476,186 @@ ctx_group_list(struct perf_event *event, struct
perf_event_context *ctx)
return &ctx->flexible_groups;
}
+static int
+perf_cpu_tree_insert(struct rb_root *tree, struct perf_event *event)
+{
+ struct rb_node **node;
+ struct rb_node *parent;
+
+ if (!tree || !event)
+ return 0;
+
+ node = &tree->rb_node;
+ parent = *node;
+
+ while (*node) {
+ struct perf_event *node_event = container_of(*node,
+ struct perf_event, group_node);
+
+ parent = *node;
+
+ if (event->cpu < node_event->cpu) {
+ node = &((*node)->rb_left);
+ } else if (event->cpu > node_event->cpu) {
+ node = &((*node)->rb_right);
+ } else {
+ list_add_tail(&event->group_list_entry,
+ &node_event->group_list);
+ return 2;
+ }
+ }
+
+ list_add_tail(&event->group_list_entry, &event->group_list);
+
+ rb_link_node(&event->group_node, parent, node);
+ rb_insert_color(&event->group_node, tree);
+
+ return 1;
+}
+
+static int
+perf_cpu_tree_delete(struct rb_root *tree, struct perf_event *event)
+{
+ if (!tree || !event)
+ return 0;
+
+ if (RB_EMPTY_NODE(&event->group_node)) {
+ list_del_init(&event->group_list_entry);
+ } else {
+ struct perf_event *list_event, *list_next;
+
+ rb_erase(&event->group_node, tree);
+ list_for_each_entry_safe(list_event, list_next,
+ &event->group_list, group_list_entry)
+ list_del_init(&list_event->group_list_entry);
+ }
+
+ return 1;
+}
+
+static struct list_head *
+perf_cpu_tree_find(struct rb_root *tree, int cpu)
+{
+ struct rb_node *node;
+
+ if (!tree)
+ return NULL;
+
+ node = tree->rb_node;
+
+ while (node) {
+ struct perf_event *node_event = container_of(node,
+ struct perf_event, group_node);
+
+ if (cpu < node_event->cpu)
+ node = node->rb_left;
+ else if (cpu > node_event->cpu)
+ node = node->rb_right;
+ else
+ return &node_event->group_list;
+ }
+
+ return NULL;
+}
+
+static struct rb_root *
+ctx_cpu_tree(struct perf_event *event, struct perf_event_context *ctx)
+{
+ if (event->attr.pinned)
+ return &ctx->pinned_tree;
+ else
+ return &ctx->flexible_tree;
+}
+
+static inline int
+event_filter_match(struct perf_event *event);
+
+static int
+group_can_go_on(struct perf_event *event,
+ struct perf_cpu_context *cpuctx,
+ int can_add_hw);
+
+static int
+group_sched_in(struct perf_event *group_event,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx);
+
+static void
+ctx_sched_in_flexible_group(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event *event, int *can_add_hw)
+{
+ if (event->state <= PERF_EVENT_STATE_OFF)
+ return;
+ if (!event_filter_match(event)) {
+ if (event->tstamp != &ctx->tstamp_data)
+ event->tstamp = &ctx->tstamp_data;
+ return;
+ }
+
+ /* may need to reset tstamp_enabled */
+ if (is_cgroup_event(event))
+ perf_cgroup_mark_enabled(event, ctx);
+
+ if (group_can_go_on(event, cpuctx, *can_add_hw)) {
+ if (group_sched_in(event, cpuctx, ctx))
+ *can_add_hw = 0;
+ }
+}
+
+static void
+ctx_sched_in_pinned_group(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event *event)
+{
+ int can_add_hw = 1;
+
+ ctx_sched_in_flexible_group(ctx, cpuctx, event, &can_add_hw);
+ if (!can_add_hw) {
+ update_group_times(event);
+ event->state = PERF_EVENT_STATE_ERROR;
+ }
+}
+
+static void
+ctx_pinned_sched_in_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ ctx_sched_in_pinned_group(ctx, cpuctx, group);
+}
+
+static void
+ctx_flexible_sched_in_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+ int can_add_hw = 1;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ ctx_sched_in_flexible_group(ctx, cpuctx, group, &can_add_hw);
+}
+
+static void
+group_sched_out(struct perf_event *group_event,
+ struct perf_cpu_context *cpuctx,
+ struct perf_event_context *ctx);
+
+static void
+ctx_sched_out_groups(struct perf_event_context *ctx,
+ struct perf_cpu_context *cpuctx,
+ struct list_head *groups)
+{
+ struct perf_event *group;
+
+ list_for_each_entry(group, groups, group_list_entry)
+ group_sched_out(group, cpuctx, ctx);
+}
+
/*
* Add a event from the lists for its context.
* Must be called with ctx->mutex and ctx->lock held.
@@ -1491,11 +1675,15 @@ list_add_event(struct perf_event *event, struct
perf_event_context *ctx)
*/
if (event->group_leader == event) {
struct list_head *list;
+ struct rb_root *tree;
event->group_caps = event->event_caps;
list = ctx_group_list(event, ctx);
list_add_tail(&event->group_entry, list);
+
+ tree = ctx_cpu_tree(event, ctx);
+ perf_cpu_tree_insert(tree, event);
}
list_update_cgroup_event(event, ctx, true);
@@ -1685,8 +1873,14 @@ list_del_event(struct perf_event *event, struct
perf_event_context *ctx)
list_del_rcu(&event->event_entry);
- if (event->group_leader == event)
+ if (event->group_leader == event) {
+ struct rb_root *tree;
+
+ tree = ctx_cpu_tree(event, ctx);
+ perf_cpu_tree_delete(tree, event);
+
list_del_init(&event->group_entry);
+ }
update_group_times(event);
@@ -1811,9 +2005,13 @@ event_sched_out(struct perf_event *event,
*/
if (event->state == PERF_EVENT_STATE_INACTIVE &&
!event_filter_match(event)) {
- delta = tstamp - event->tstamp_stopped;
- event->tstamp_running += delta;
- event->tstamp_stopped = tstamp;
+ delta = tstamp - event->tstamp->stopped;
+ event->tstamp->running += delta;
+ event->tstamp->stopped = tstamp;
+ if (event->tstamp != &event->tstamp_data) {
+ event->tstamp_data = *event->tstamp;
+ event->tstamp = &event->tstamp_data;
+ }
}
if (event->state != PERF_EVENT_STATE_ACTIVE)
@@ -1821,7 +2019,7 @@ event_sched_out(struct perf_event *event,
perf_pmu_disable(event->pmu);
- event->tstamp_stopped = tstamp;
+ event->tstamp->stopped = tstamp;
event->pmu->del(event, 0);
event->oncpu = -1;
event->state = PERF_EVENT_STATE_INACTIVE;
@@ -2096,7 +2294,7 @@ event_sched_in(struct perf_event *event,
goto out;
}
- event->tstamp_running += tstamp - event->tstamp_stopped;
+ event->tstamp->running += tstamp - event->tstamp->stopped;
if (!is_software_event(event))
cpuctx->active_oncpu++;
@@ -2168,8 +2366,8 @@ group_sched_in(struct perf_event *group_event,
simulate = true;
if (simulate) {
- event->tstamp_running += now - event->tstamp_stopped;
- event->tstamp_stopped = now;
+ event->tstamp->running += now - event->tstamp->stopped;
+ event->tstamp->stopped = now;
} else {
event_sched_out(event, cpuctx, ctx);
}
@@ -2221,43 +2419,45 @@ static void add_event_to_ctx(struct perf_event
*event,
list_add_event(event, ctx);
perf_group_attach(event);
- event->tstamp_enabled = tstamp;
- event->tstamp_running = tstamp;
- event->tstamp_stopped = tstamp;
+ event->tstamp->enabled = tstamp;
+ event->tstamp->running = tstamp;
+ event->tstamp->stopped = tstamp;
}
static void ctx_sched_out(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
- enum event_type_t event_type);
+ enum event_type_t event_type, int mux);
static void
ctx_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task);
+ struct task_struct *task, int mux);
static void task_ctx_sched_out(struct perf_cpu_context *cpuctx,
struct perf_event_context *ctx,
enum event_type_t event_type)
{
+ int mux = 0;
+
if (!cpuctx->task_ctx)
return;
if (WARN_ON_ONCE(ctx != cpuctx->task_ctx))
return;
- ctx_sched_out(ctx, cpuctx, event_type);
+ ctx_sched_out(ctx, cpuctx, event_type, mux);
}
static void perf_event_sched_in(struct perf_cpu_context *cpuctx,
struct perf_event_context *ctx,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
- cpu_ctx_sched_in(cpuctx, EVENT_PINNED, task);
+ cpu_ctx_sched_in(cpuctx, EVENT_PINNED, task, mux);
if (ctx)
- ctx_sched_in(ctx, cpuctx, EVENT_PINNED, task);
- cpu_ctx_sched_in(cpuctx, EVENT_FLEXIBLE, task);
+ ctx_sched_in(ctx, cpuctx, EVENT_PINNED, task, mux);
+ cpu_ctx_sched_in(cpuctx, EVENT_FLEXIBLE, task, mux);
if (ctx)
- ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task);
+ ctx_sched_in(ctx, cpuctx, EVENT_FLEXIBLE, task, mux);
}
/*
@@ -2281,6 +2481,7 @@ static void ctx_resched(struct perf_cpu_context
*cpuctx,
{
enum event_type_t ctx_event_type = event_type & EVENT_ALL;
bool cpu_event = !!(event_type & EVENT_CPU);
+ int mux = 0;
/*
* If pinned groups are involved, flexible groups also need to be
@@ -2301,11 +2502,11 @@ static void ctx_resched(struct perf_cpu_context
*cpuctx,
* - otherwise, do nothing more.
*/
if (cpu_event)
- cpu_ctx_sched_out(cpuctx, ctx_event_type);
+ cpu_ctx_sched_out(cpuctx, ctx_event_type, mux);
else if (ctx_event_type & EVENT_PINNED)
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
- perf_event_sched_in(cpuctx, task_ctx, current);
+ perf_event_sched_in(cpuctx, task_ctx, current, mux);
perf_pmu_enable(cpuctx->ctx.pmu);
}
@@ -2323,6 +2524,7 @@ static int __perf_install_in_context(void *info)
struct perf_event_context *task_ctx = cpuctx->task_ctx;
bool reprogram = true;
int ret = 0;
+ int mux = 0;
raw_spin_lock(&cpuctx->ctx.lock);
if (ctx->task) {
@@ -2349,7 +2551,7 @@ static int __perf_install_in_context(void *info)
}
if (reprogram) {
- ctx_sched_out(ctx, cpuctx, EVENT_TIME);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
add_event_to_ctx(event, ctx);
ctx_resched(cpuctx, task_ctx, get_event_type(event));
} else {
@@ -2468,10 +2670,10 @@ static void __perf_event_mark_enabled(struct
perf_event *event)
u64 tstamp = perf_event_time(event);
event->state = PERF_EVENT_STATE_INACTIVE;
- event->tstamp_enabled = tstamp - event->total_time_enabled;
+ event->tstamp->enabled = tstamp - event->total_time_enabled;
list_for_each_entry(sub, &event->sibling_list, group_entry) {
if (sub->state >= PERF_EVENT_STATE_INACTIVE)
- sub->tstamp_enabled = tstamp - sub->total_time_enabled;
+ sub->tstamp->enabled = tstamp - sub->total_time_enabled;
}
}
@@ -2485,13 +2687,14 @@ static void __perf_event_enable(struct
perf_event *event,
{
struct perf_event *leader = event->group_leader;
struct perf_event_context *task_ctx;
+ int mux = 0;
if (event->state >= PERF_EVENT_STATE_INACTIVE ||
event->state <= PERF_EVENT_STATE_ERROR)
return;
if (ctx->is_active)
- ctx_sched_out(ctx, cpuctx, EVENT_TIME);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
__perf_event_mark_enabled(event);
@@ -2501,7 +2704,7 @@ static void __perf_event_enable(struct perf_event
*event,
if (!event_filter_match(event)) {
if (is_cgroup_event(event))
perf_cgroup_defer_enabled(event);
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
return;
}
@@ -2510,7 +2713,7 @@ static void __perf_event_enable(struct perf_event
*event,
* then don't put it on unless the group is on.
*/
if (leader != event && leader->state != PERF_EVENT_STATE_ACTIVE) {
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
return;
}
@@ -2706,10 +2909,12 @@ EXPORT_SYMBOL_GPL(perf_event_refresh);
static void ctx_sched_out(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
- enum event_type_t event_type)
+ enum event_type_t event_type, int mux)
{
int is_active = ctx->is_active;
struct perf_event *event;
+ struct list_head *groups;
+ int cpu = smp_processor_id();
lockdep_assert_held(&ctx->lock);
@@ -2756,13 +2961,34 @@ static void ctx_sched_out(struct
perf_event_context *ctx,
perf_pmu_disable(ctx->pmu);
if (is_active & EVENT_PINNED) {
- list_for_each_entry(event, &ctx->pinned_groups, group_entry)
- group_sched_out(event, cpuctx, ctx);
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, -1);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, cpu);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ } else {
+ list_for_each_entry(event, &ctx->pinned_groups,
+ group_entry)
+ group_sched_out(event, cpuctx, ctx);
+ }
}
if (is_active & EVENT_FLEXIBLE) {
- list_for_each_entry(event, &ctx->flexible_groups, group_entry)
- group_sched_out(event, cpuctx, ctx);
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, cpu);
+ if (groups)
+ ctx_sched_out_groups(ctx, cpuctx, groups);
+
+ } else {
+ list_for_each_entry(event, &ctx->flexible_groups,
+ group_entry)
+ group_sched_out(event, cpuctx, ctx);
+ }
}
perf_pmu_enable(ctx->pmu);
}
@@ -3051,9 +3277,9 @@ void __perf_event_task_sched_out(struct
task_struct *task,
* Called with IRQs disabled
*/
static void cpu_ctx_sched_out(struct perf_cpu_context *cpuctx,
- enum event_type_t event_type)
+ enum event_type_t event_type, int mux)
{
- ctx_sched_out(&cpuctx->ctx, cpuctx, event_type);
+ ctx_sched_out(&cpuctx->ctx, cpuctx, event_type, mux);
}
static void
@@ -3061,29 +3287,8 @@ ctx_pinned_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx)
{
struct perf_event *event;
-
- list_for_each_entry(event, &ctx->pinned_groups, group_entry) {
- if (event->state <= PERF_EVENT_STATE_OFF)
- continue;
- if (!event_filter_match(event))
- continue;
-
- /* may need to reset tstamp_enabled */
- if (is_cgroup_event(event))
- perf_cgroup_mark_enabled(event, ctx);
-
- if (group_can_go_on(event, cpuctx, 1))
- group_sched_in(event, cpuctx, ctx);
-
- /*
- * If this pinned group hasn't been scheduled,
- * put it in error state.
- */
- if (event->state == PERF_EVENT_STATE_INACTIVE) {
- update_group_times(event);
- event->state = PERF_EVENT_STATE_ERROR;
- }
- }
+ list_for_each_entry(event, &ctx->pinned_groups, group_entry)
+ ctx_sched_in_pinned_group(ctx, cpuctx, event);
}
static void
@@ -3092,37 +3297,19 @@ ctx_flexible_sched_in(struct perf_event_context
*ctx,
{
struct perf_event *event;
int can_add_hw = 1;
-
- list_for_each_entry(event, &ctx->flexible_groups, group_entry) {
- /* Ignore events in OFF or ERROR state */
- if (event->state <= PERF_EVENT_STATE_OFF)
- continue;
- /*
- * Listen to the 'cpu' scheduling filter constraint
- * of events:
- */
- if (!event_filter_match(event))
- continue;
-
- /* may need to reset tstamp_enabled */
- if (is_cgroup_event(event))
- perf_cgroup_mark_enabled(event, ctx);
-
- if (group_can_go_on(event, cpuctx, can_add_hw)) {
- if (group_sched_in(event, cpuctx, ctx))
- can_add_hw = 0;
- }
- }
+ list_for_each_entry(event, &ctx->flexible_groups, group_entry)
+ ctx_sched_in_flexible_group(ctx, cpuctx, event, &can_add_hw);
}
static void
ctx_sched_in(struct perf_event_context *ctx,
struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
int is_active = ctx->is_active;
- u64 now;
+ struct list_head *groups;
+ int cpu = smp_processor_id();
lockdep_assert_held(&ctx->lock);
@@ -3141,8 +3328,7 @@ ctx_sched_in(struct perf_event_context *ctx,
if (is_active & EVENT_TIME) {
/* start ctx time */
- now = perf_clock();
- ctx->timestamp = now;
+ ctx->timestamp = perf_clock();
perf_cgroup_set_timestamp(task, ctx);
}
@@ -3150,27 +3336,50 @@ ctx_sched_in(struct perf_event_context *ctx,
* First go through the list and put on any pinned groups
* in order to give them the best chance of going on.
*/
- if (is_active & EVENT_PINNED)
- ctx_pinned_sched_in(ctx, cpuctx);
+ if (is_active & EVENT_PINNED) {
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, -1);
+ if (groups)
+ ctx_pinned_sched_in_groups(ctx, cpuctx, groups);
+ groups = perf_cpu_tree_find(&ctx->pinned_tree, cpu);
+ if (groups)
+ ctx_pinned_sched_in_groups(ctx, cpuctx, groups);
+ } else {
+ ctx_pinned_sched_in(ctx, cpuctx);
+ }
+ }
/* Then walk through the lower prio flexible groups */
- if (is_active & EVENT_FLEXIBLE)
- ctx_flexible_sched_in(ctx, cpuctx);
+ if (is_active & EVENT_FLEXIBLE) {
+ if (mux) {
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ ctx_flexible_sched_in_groups(ctx, cpuctx,
+ groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, cpu);
+ if (groups)
+ ctx_flexible_sched_in_groups(ctx, cpuctx,
+ groups);
+ } else {
+ ctx_flexible_sched_in(ctx, cpuctx);
+ }
+ }
}
static void cpu_ctx_sched_in(struct perf_cpu_context *cpuctx,
enum event_type_t event_type,
- struct task_struct *task)
+ struct task_struct *task, int mux)
{
struct perf_event_context *ctx = &cpuctx->ctx;
- ctx_sched_in(ctx, cpuctx, event_type, task);
+ ctx_sched_in(ctx, cpuctx, event_type, task, mux);
}
static void perf_event_context_sched_in(struct perf_event_context *ctx,
struct task_struct *task)
{
struct perf_cpu_context *cpuctx;
+ int mux = 0;
cpuctx = __get_cpu_context(ctx);
if (cpuctx->task_ctx == ctx)
@@ -3187,8 +3396,8 @@ static void perf_event_context_sched_in(struct
perf_event_context *ctx,
* events, no need to flip the cpuctx's events around.
*/
if (!list_empty(&ctx->pinned_groups))
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
- perf_event_sched_in(cpuctx, ctx, task);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
+ perf_event_sched_in(cpuctx, ctx, task, mux);
perf_pmu_enable(ctx->pmu);
perf_ctx_unlock(cpuctx, ctx);
}
@@ -3421,14 +3630,24 @@ static void rotate_ctx(struct perf_event_context
*ctx)
* Rotate the first entry last of non-pinned groups. Rotation might be
* disabled by the inheritance code.
*/
- if (!ctx->rotate_disable)
- list_rotate_left(&ctx->flexible_groups);
+ if (!ctx->rotate_disable) {
+ struct list_head *groups;
+
+ groups = perf_cpu_tree_find(&ctx->flexible_tree, -1);
+ if (groups)
+ list_rotate_left(groups);
+ groups = perf_cpu_tree_find(&ctx->flexible_tree,
+ smp_processor_id());
+ if (groups)
+ list_rotate_left(groups);
+ }
}
static int perf_rotate_context(struct perf_cpu_context *cpuctx)
{
struct perf_event_context *ctx = NULL;
int rotate = 0;
+ int mux = 1;
if (cpuctx->ctx.nr_events) {
if (cpuctx->ctx.nr_events != cpuctx->ctx.nr_active)
@@ -3447,15 +3666,15 @@ static int perf_rotate_context(struct
perf_cpu_context *cpuctx)
perf_ctx_lock(cpuctx, cpuctx->task_ctx);
perf_pmu_disable(cpuctx->ctx.pmu);
- cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
+ cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE, mux);
if (ctx)
- ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE);
+ ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE, mux);
rotate_ctx(&cpuctx->ctx);
if (ctx)
rotate_ctx(ctx);
- perf_event_sched_in(cpuctx, ctx, current);
+ perf_event_sched_in(cpuctx, ctx, current, mux);
perf_pmu_enable(cpuctx->ctx.pmu);
perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
@@ -3507,6 +3726,7 @@ static void perf_event_enable_on_exec(int ctxn)
struct perf_event *event;
unsigned long flags;
int enabled = 0;
+ int mux = 0;
local_irq_save(flags);
ctx = current->perf_event_ctxp[ctxn];
@@ -3515,7 +3735,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);
+ ctx_sched_out(ctx, cpuctx, EVENT_TIME, mux);
list_for_each_entry(event, &ctx->event_list, event_entry) {
enabled |= event_enable_on_exec(event, ctx);
event_type |= get_event_type(event);
@@ -3528,7 +3748,7 @@ static void perf_event_enable_on_exec(int ctxn)
clone_ctx = unclone_ctx(ctx);
ctx_resched(cpuctx, ctx, event_type);
} else {
- ctx_sched_in(ctx, cpuctx, EVENT_TIME, current);
+ ctx_sched_in(ctx, cpuctx, EVENT_TIME, current, mux);
}
perf_ctx_unlock(cpuctx, ctx);
@@ -3749,7 +3969,9 @@ static void __perf_event_init_context(struct
perf_event_context *ctx)
mutex_init(&ctx->mutex);
INIT_LIST_HEAD(&ctx->active_ctx_list);
INIT_LIST_HEAD(&ctx->pinned_groups);
+ ctx->pinned_tree = RB_ROOT;
INIT_LIST_HEAD(&ctx->flexible_groups);
+ ctx->flexible_tree = RB_ROOT;
INIT_LIST_HEAD(&ctx->event_list);
atomic_set(&ctx->refcount, 1);
}
@@ -4848,8 +5070,8 @@ static void calc_timer_values(struct perf_event
*event,
*now = perf_clock();
ctx_time = event->shadow_ctx_time + *now;
- *enabled = ctx_time - event->tstamp_enabled;
- *running = ctx_time - event->tstamp_running;
+ *enabled = ctx_time - event->tstamp->enabled;
+ *running = ctx_time - event->tstamp->running;
}
static void perf_event_init_userpage(struct perf_event *event)
@@ -9357,6 +9579,9 @@ perf_event_alloc(struct perf_event_attr *attr, int
cpu,
INIT_LIST_HEAD(&event->child_list);
INIT_LIST_HEAD(&event->group_entry);
+ RB_CLEAR_NODE(&event->group_node);
+ INIT_LIST_HEAD(&event->group_list);
+ INIT_LIST_HEAD(&event->group_list_entry);
INIT_LIST_HEAD(&event->event_entry);
INIT_LIST_HEAD(&event->sibling_list);
INIT_LIST_HEAD(&event->rb_entry);
@@ -9372,6 +9597,7 @@ perf_event_alloc(struct perf_event_attr *attr, int
cpu,
raw_spin_lock_init(&event->addr_filters.lock);
atomic_long_set(&event->refcount, 1);
+ event->tstamp = &event->tstamp_data;
event->cpu = cpu;
event->attr = *attr;
event->group_leader = group_leader;
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 09:50 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMnB7-56h-1@gated-at.bofh.it> |
| In reply to | #1651863 |
On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote:
> @@ -571,6 +587,27 @@ struct perf_event {
> * either sufficies for read.
> */
> struct list_head group_entry;
> + /*
> + * Node on the pinned or flexible tree located at the event context;
> + * the node may be empty in case its event is not directly attached
> + * to the tree but to group_list list of the event directly
> + * attached to the tree;
> + */
> + struct rb_node group_node;
> + /*
> + * List keeps groups allocated for the same cpu;
> + * the list may be empty in case its event is not directly
> + * attached to the tree but to group_list list of the event directly
> + * attached to the tree;
> + */
> + struct list_head group_list;
> + /*
> + * Entry into the group_list list above;
> + * the entry may be attached to the self group_list list above
> + * in case the event is directly attached to the pinned or
> + * flexible tree;
> + */
> + struct list_head group_list_entry;
> struct list_head sibling_list;
>
> /*
> @@ -742,7 +772,17 @@ struct perf_event_context {
>
> struct list_head active_ctx_list;
> struct list_head pinned_groups;
> + /*
> + * Cpu tree for pinned groups; keeps event's group_node nodes
> + * of attached flexible groups;
> + */
> + struct rb_root pinned_tree;
> struct list_head flexible_groups;
> + /*
> + * Cpu tree for flexible groups; keeps event's group_node nodes
> + * of attached flexible groups;
> + */
> + struct rb_root flexible_tree;
> struct list_head event_list;
> int nr_events;
> int nr_active;
> @@ -758,6 +798,7 @@ struct perf_event_context {
> */
> u64 time;
> u64 timestamp;
> + struct perf_event_tstamp tstamp_data;
>
> /*
> * These fields let us detect when two contexts have both
So why do we now have a list _and_ a tree for the same entries?
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 11:20 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMp0f-6bg-47@gated-at.bofh.it> |
| In reply to | #1652256 |
On 29.05.2017 10:46, Peter Zijlstra wrote:
> On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote:
>> @@ -571,6 +587,27 @@ struct perf_event {
>> * either sufficies for read.
>> */
>> struct list_head group_entry;
>> + /*
>> + * Node on the pinned or flexible tree located at the event context;
>> + * the node may be empty in case its event is not directly attached
>> + * to the tree but to group_list list of the event directly
>> + * attached to the tree;
>> + */
>> + struct rb_node group_node;
>> + /*
>> + * List keeps groups allocated for the same cpu;
>> + * the list may be empty in case its event is not directly
>> + * attached to the tree but to group_list list of the event directly
>> + * attached to the tree;
>> + */
>> + struct list_head group_list;
>> + /*
>> + * Entry into the group_list list above;
>> + * the entry may be attached to the self group_list list above
>> + * in case the event is directly attached to the pinned or
>> + * flexible tree;
>> + */
>> + struct list_head group_list_entry;
>> struct list_head sibling_list;
>>
>> /*
>
>> @@ -742,7 +772,17 @@ struct perf_event_context {
>>
>> struct list_head active_ctx_list;
>> struct list_head pinned_groups;
>> + /*
>> + * Cpu tree for pinned groups; keeps event's group_node nodes
>> + * of attached flexible groups;
>> + */
>> + struct rb_root pinned_tree;
>> struct list_head flexible_groups;
>> + /*
>> + * Cpu tree for flexible groups; keeps event's group_node nodes
>> + * of attached flexible groups;
>> + */
>> + struct rb_root flexible_tree;
>> struct list_head event_list;
>> int nr_events;
>> int nr_active;
>> @@ -758,6 +798,7 @@ struct perf_event_context {
>> */
>> u64 time;
>> u64 timestamp;
>> + struct perf_event_tstamp tstamp_data;
>>
>> /*
>> * These fields let us detect when two contexts have both
>
>
> So why do we now have a list _and_ a tree for the same entries?
We need groups list to iterate through all groups configured for
collection and we need the tree to quickly iterate through the groups
allocated for a particular CPU only.
>
>
-Alexey
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 12:50 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMqpk-72M-15@gated-at.bofh.it> |
| In reply to | #1652343 |
On Mon, May 29, 2017 at 12:15:14PM +0300, Alexey Budankov wrote:
> On 29.05.2017 10:46, Peter Zijlstra wrote:
> > On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote:
> > > @@ -742,7 +772,17 @@ struct perf_event_context {
> > >
> > > struct list_head active_ctx_list;
> > > struct list_head pinned_groups;
> > > + /*
> > > + * Cpu tree for pinned groups; keeps event's group_node nodes
> > > + * of attached flexible groups;
> > > + */
> > > + struct rb_root pinned_tree;
> > > struct list_head flexible_groups;
> > > + /*
> > > + * Cpu tree for flexible groups; keeps event's group_node nodes
> > > + * of attached flexible groups;
> > > + */
> > > + struct rb_root flexible_tree;
> > > struct list_head event_list;
> > > int nr_events;
> > > int nr_active;
> > > @@ -758,6 +798,7 @@ struct perf_event_context {
> > > */
> > > u64 time;
> > > u64 timestamp;
> > > + struct perf_event_tstamp tstamp_data;
> > >
> > > /*
> > > * These fields let us detect when two contexts have both
> >
> >
> > So why do we now have a list _and_ a tree for the same entries?
> We need groups list to iterate through all groups configured for collection
> and we need the tree to quickly iterate through the groups allocated for a
> particular CPU only.
*confused*, what?
Why can't the tree do both?
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 13:00 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMqz0-77t-11@gated-at.bofh.it> |
| In reply to | #1652408 |
On 29.05.2017 13:43, Peter Zijlstra wrote:
> On Mon, May 29, 2017 at 12:15:14PM +0300, Alexey Budankov wrote:
>> On 29.05.2017 10:46, Peter Zijlstra wrote:
>>> On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote:
>
>>>> @@ -742,7 +772,17 @@ struct perf_event_context {
>>>>
>>>> struct list_head active_ctx_list;
>>>> struct list_head pinned_groups;
>>>> + /*
>>>> + * Cpu tree for pinned groups; keeps event's group_node nodes
>>>> + * of attached flexible groups;
>>>> + */
>>>> + struct rb_root pinned_tree;
>>>> struct list_head flexible_groups;
>>>> + /*
>>>> + * Cpu tree for flexible groups; keeps event's group_node nodes
>>>> + * of attached flexible groups;
>>>> + */
>>>> + struct rb_root flexible_tree;
>>>> struct list_head event_list;
>>>> int nr_events;
>>>> int nr_active;
>>>> @@ -758,6 +798,7 @@ struct perf_event_context {
>>>> */
>>>> u64 time;
>>>> u64 timestamp;
>>>> + struct perf_event_tstamp tstamp_data;
>>>>
>>>> /*
>>>> * These fields let us detect when two contexts have both
>>>
>>>
>>> So why do we now have a list _and_ a tree for the same entries?
>
>> We need groups list to iterate through all groups configured for collection
>> and we need the tree to quickly iterate through the groups allocated for a
>> particular CPU only.
>
> *confused*, what?
>
> Why can't the tree do both?
>
Well, indeed, the tree provides such capability too. However switching
to the full tree iteration in cases where we now go through _groups
lists will enlarge the patch, what is probably is not a big deal. Do you
think it is worth implementing the switch?
Thanks,
Alexey
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 13:30 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMr22-7Aq-23@gated-at.bofh.it> |
| In reply to | #1652416 |
On Mon, May 29, 2017 at 01:56:05PM +0300, Alexey Budankov wrote: > On 29.05.2017 13:43, Peter Zijlstra wrote: > > Why can't the tree do both? > > > > Well, indeed, the tree provides such capability too. However switching to > the full tree iteration in cases where we now go through _groups lists will > enlarge the patch, what is probably is not a big deal. Do you think it is > worth implementing the switch? Do it as a series of patches, where patch 1 introduces the tree, patches 2 through n convert the list users into tree users, and patch n+1 removes the list. I think its good to not have duplicate data structures if we can avoid it.
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 13:50 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMrlo-7Ii-17@gated-at.bofh.it> |
| In reply to | #1652430 |
On 29.05.2017 14:23, Peter Zijlstra wrote: > On Mon, May 29, 2017 at 01:56:05PM +0300, Alexey Budankov wrote: >> On 29.05.2017 13:43, Peter Zijlstra wrote: > >>> Why can't the tree do both? >>> >> >> Well, indeed, the tree provides such capability too. However switching to >> the full tree iteration in cases where we now go through _groups lists will >> enlarge the patch, what is probably is not a big deal. Do you think it is >> worth implementing the switch? > > Do it as a series of patches, where patch 1 introduces the tree, patches > 2 through n convert the list users into tree users, and patch n+1 > removes the list. Well ok, let's do that additionally but please expect delay in delivery (I am OOO till Jun 14). > > I think its good to not have duplicate data structures if we can avoid > it. > yeah, makes sense.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 09:50 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMnB7-56h-7@gated-at.bofh.it> |
| In reply to | #1651863 |
On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote: > Solution: > > cpu indexed trees for perf_event_context::pinned_groups and > perf_event_context::flexible_groups lists are introduced. Every tree node > keeps a list of groups allocated for the same cpu. A tree references only > groups located at the appropriate group list. The tree provides capability > to iterate over groups allocated for a specific cpu only, what is exactly > required by multiplexing timer interrupt handler. The handler runs per-cpu > and enables/disables groups using group_sched_in()/group_sched_out() that > call event_filter_match() function filtering out groups allocated for cpus > different from the one executing the handler. Additionally for every > filtered out group group_sched_out() updates tstamps values to the current > interrupt time. This updating work is now done only once by > update_context_time() called by ctx_sched_out() before cpu groups > iteration. For this trick to work it is required that tstamps of filtered > out events would point to perf_event_context::tstamp_data object instead > of perf_event::tstamp_data ones, as it is initialized from an event > allocation. tstamps references are switched by > group_sched_in()/group_sched_out() every time a group is checked for its > suitability for currently running cpu. When a thread enters some cpu on > a context switch a long run through pinned and flexible groups is > performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0 > and filtered out groups tstamps are switched to > perf_event_context::tstamp_data object. Then a series of multiplexing > interrupts happens and the handler rotates the flexible groups calling > ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu > tree lists only and avoiding long runs through the complete group lists. > This is where speedup comes from. Eventually when the thread leaves the cpu > ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events' > perf_event::tstamp_data objects. This is unreadable.. Please use whitespace. > Added API: > > Objects: > 1. struct perf_event_tstamp: > - enabled > - running > - stopped > 2. struct perf_event: > - group_node > - group_list > - group_list_entry > - tstamp > - tstamp_data > 3. struct perf_event_context: > - pinned_tree > - flexible_tree > - tstamp_data > > Functions: > 1. insert a group into a tree using event->cpu as a key: > int perf_cpu_tree_insert(struct rb_root *tree, > struct perf_event *event); > 2. delete a group from a tree, if the group is directly attached > to a tree it also detaches all groups on the groups > group_list list: > int perf_cpu_tree_delete(struct rb_root *tree, > struct perf_event *event); > 3. find group_list list by a cpu key: > struct list_head * perf_cpu_tree_find(struct rb_root *tree, > int cpu); > 4. enable a pinned group on a cpu: > void ctx_sched_in_flexible_group(struct perf_event_context *ctx, > struct perf_cpu_context *cpuctx, > struct perf_event *group, int *can_add_hw); > 5. enable a flexible group on a cpu; calls ctx_sched_in_flexible_group > and updates group->state to ERROR in case of failure: > void ctx_sched_in_pinned_group(struct perf_event_context *ctx, > struct perf_cpu_context *cpuctx, > struct perf_event *group); > 6. enable per-cpu pinned tree's groups on a cpu: > void ctx_pinned_sched_in_groups(struct perf_event_context *ctx, > struct perf_cpu_context *cpuctx, > struct list_head *groups); > 7. enable per-cpu flexible tree's groups on a cpu: > void ctx_flexible_sched_in_groups( > struct perf_event_context *ctx, > struct perf_cpu_context *cpuctx, > struct list_head *groups); > 8. disable per-cpu tree's groups on a cpu: > void ctx_sched_out_groups(struct perf_event_context *ctx, > struct perf_cpu_context *cpuctx, > struct list_head *groups); > 9. get a group tree based on event->attr.pinned attribute value: > struct rb_root * ctx_cpu_tree(struct perf_event *event, > struct perf_event_context *ctx); > > Modified API: > > Objects: > 1. struct perf_event > 2. struct perf_event_context > > Functions: > 1. perf_event_alloc() > 2. add_event_to_ctx() > 3. perf_event_enable_on_exec() > 4. __perf_install_in_context() > 5. __perf_event_init_context() > 6. __perf_event_mark_enabled() > 7. __perf_event_enable() > 8. __perf_event_task_sched_out() > 9. ctx_group_list() > 10. list_add_event() > 11. list_del_event() > 12. perf_event_context_sched_in() > 13. cpu_ctx_sched_in() > 14. cpu_ctx_sched_out() > 15. ctx_sched_in() > 16. ctx_sched_out() > 17. ctx_resched() > 18. ctx_pinned_sched_in() > 19. ctx_flexible_sched_in() > 20. group_sched_in() > 21. event_sched_in() > 22. event_sched_out() > 23. rotate_ctx() > 24. perf_rotate_context() > 25. update_context_time() > 26. update_event_times() > 27. calc_timer_values() > 28. perf_cgroup_switch() > 29. perf_cgroup_mark_enabled() Yeah, this doesn't go into a changelog. Have you _ever_ seen a changelog with such crud in?
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 11:30 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMp9U-6ft-21@gated-at.bofh.it> |
| In reply to | #1652258 |
On 29.05.2017 10:45, Peter Zijlstra wrote: > On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote: >> Solution: >> >> cpu indexed trees for perf_event_context::pinned_groups and >> perf_event_context::flexible_groups lists are introduced. Every tree node >> keeps a list of groups allocated for the same cpu. A tree references only >> groups located at the appropriate group list. The tree provides capability >> to iterate over groups allocated for a specific cpu only, what is exactly >> required by multiplexing timer interrupt handler. The handler runs per-cpu >> and enables/disables groups using group_sched_in()/group_sched_out() that >> call event_filter_match() function filtering out groups allocated for cpus >> different from the one executing the handler. Additionally for every >> filtered out group group_sched_out() updates tstamps values to the current >> interrupt time. This updating work is now done only once by >> update_context_time() called by ctx_sched_out() before cpu groups >> iteration. For this trick to work it is required that tstamps of filtered >> out events would point to perf_event_context::tstamp_data object instead >> of perf_event::tstamp_data ones, as it is initialized from an event >> allocation. tstamps references are switched by >> group_sched_in()/group_sched_out() every time a group is checked for its >> suitability for currently running cpu. When a thread enters some cpu on >> a context switch a long run through pinned and flexible groups is >> performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0 >> and filtered out groups tstamps are switched to >> perf_event_context::tstamp_data object. Then a series of multiplexing >> interrupts happens and the handler rotates the flexible groups calling >> ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu >> tree lists only and avoiding long runs through the complete group lists. >> This is where speedup comes from. Eventually when the thread leaves the cpu >> ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events' >> perf_event::tstamp_data objects. > > This is unreadable.. Please use whitespace. Do you mean do NOT use whitespaces? Could you explain in more detail what you mean? > > >> Added API: >> >> Objects: >> 1. struct perf_event_tstamp: >> - enabled >> - running >> - stopped >> 2. struct perf_event: >> - group_node >> - group_list >> - group_list_entry >> - tstamp >> - tstamp_data >> 3. struct perf_event_context: >> - pinned_tree >> - flexible_tree >> - tstamp_data >> >> Functions: >> 1. insert a group into a tree using event->cpu as a key: >> int perf_cpu_tree_insert(struct rb_root *tree, >> struct perf_event *event); >> 2. delete a group from a tree, if the group is directly attached >> to a tree it also detaches all groups on the groups >> group_list list: >> int perf_cpu_tree_delete(struct rb_root *tree, >> struct perf_event *event); >> 3. find group_list list by a cpu key: >> struct list_head * perf_cpu_tree_find(struct rb_root *tree, >> int cpu); >> 4. enable a pinned group on a cpu: >> void ctx_sched_in_flexible_group(struct perf_event_context *ctx, >> struct perf_cpu_context *cpuctx, >> struct perf_event *group, int *can_add_hw); >> 5. enable a flexible group on a cpu; calls ctx_sched_in_flexible_group >> and updates group->state to ERROR in case of failure: >> void ctx_sched_in_pinned_group(struct perf_event_context *ctx, >> struct perf_cpu_context *cpuctx, >> struct perf_event *group); >> 6. enable per-cpu pinned tree's groups on a cpu: >> void ctx_pinned_sched_in_groups(struct perf_event_context *ctx, >> struct perf_cpu_context *cpuctx, >> struct list_head *groups); >> 7. enable per-cpu flexible tree's groups on a cpu: >> void ctx_flexible_sched_in_groups( >> struct perf_event_context *ctx, >> struct perf_cpu_context *cpuctx, >> struct list_head *groups); >> 8. disable per-cpu tree's groups on a cpu: >> void ctx_sched_out_groups(struct perf_event_context *ctx, >> struct perf_cpu_context *cpuctx, >> struct list_head *groups); >> 9. get a group tree based on event->attr.pinned attribute value: >> struct rb_root * ctx_cpu_tree(struct perf_event *event, >> struct perf_event_context *ctx); >> >> Modified API: >> >> Objects: >> 1. struct perf_event >> 2. struct perf_event_context >> >> Functions: >> 1. perf_event_alloc() >> 2. add_event_to_ctx() >> 3. perf_event_enable_on_exec() >> 4. __perf_install_in_context() >> 5. __perf_event_init_context() >> 6. __perf_event_mark_enabled() >> 7. __perf_event_enable() >> 8. __perf_event_task_sched_out() >> 9. ctx_group_list() >> 10. list_add_event() >> 11. list_del_event() >> 12. perf_event_context_sched_in() >> 13. cpu_ctx_sched_in() >> 14. cpu_ctx_sched_out() >> 15. ctx_sched_in() >> 16. ctx_sched_out() >> 17. ctx_resched() >> 18. ctx_pinned_sched_in() >> 19. ctx_flexible_sched_in() >> 20. group_sched_in() >> 21. event_sched_in() >> 22. event_sched_out() >> 23. rotate_ctx() >> 24. perf_rotate_context() >> 25. update_context_time() >> 26. update_event_times() >> 27. calc_timer_values() >> 28. perf_cgroup_switch() >> 29. perf_cgroup_mark_enabled() > > Yeah, this doesn't go into a changelog. Have you _ever_ seen a changelog > with such crud in? > I have not. Could you please advise how to format this information to be suitable for changelog, if it's required at all? Thanks, Alexey
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 12:40 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMqfE-6Yv-9@gated-at.bofh.it> |
| In reply to | #1652359 |
On Mon, May 29, 2017 at 12:24:53PM +0300, Alexey Budankov wrote: > On 29.05.2017 10:45, Peter Zijlstra wrote: > > On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote: > > > Solution: > > > > > > cpu indexed trees for perf_event_context::pinned_groups and > > > perf_event_context::flexible_groups lists are introduced. Every tree node > > > keeps a list of groups allocated for the same cpu. A tree references only > > > groups located at the appropriate group list. The tree provides capability > > > to iterate over groups allocated for a specific cpu only, what is exactly > > > required by multiplexing timer interrupt handler. The handler runs per-cpu > > > and enables/disables groups using group_sched_in()/group_sched_out() that > > > call event_filter_match() function filtering out groups allocated for cpus > > > different from the one executing the handler. Additionally for every > > > filtered out group group_sched_out() updates tstamps values to the current > > > interrupt time. This updating work is now done only once by > > > update_context_time() called by ctx_sched_out() before cpu groups > > > iteration. For this trick to work it is required that tstamps of filtered > > > out events would point to perf_event_context::tstamp_data object instead > > > of perf_event::tstamp_data ones, as it is initialized from an event > > > allocation. tstamps references are switched by > > > group_sched_in()/group_sched_out() every time a group is checked for its > > > suitability for currently running cpu. When a thread enters some cpu on > > > a context switch a long run through pinned and flexible groups is > > > performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0 > > > and filtered out groups tstamps are switched to > > > perf_event_context::tstamp_data object. Then a series of multiplexing > > > interrupts happens and the handler rotates the flexible groups calling > > > ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu > > > tree lists only and avoiding long runs through the complete group lists. > > > This is where speedup comes from. Eventually when the thread leaves the cpu > > > ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events' > > > perf_event::tstamp_data objects. > > > > This is unreadable.. Please use whitespace. > > Do you mean do NOT use whitespaces? Could you explain in more detail what > you mean? No, add _more_ whitespace. Use things like paragraphs and such. Reading a massive blob of text like that is painful. Also use full and complete sentences. For example, the very first sentence: 'cpu indexed trees for perf_event_context::pinned_groups and perf_event_context::flexible_groups lists are introduced.' feels incomplete and leaves one wondering what for etc.. And it only gets worse. > > Yeah, this doesn't go into a changelog. Have you _ever_ seen a changelog > > with such crud in? > > > > I have not. Could you please advise how to format this information to be > suitable for changelog, if it's required at all? Don't include it. It should be fairly obvious from the diff itself what changed after all.
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 12:50 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMqpk-72M-17@gated-at.bofh.it> |
| In reply to | #1652404 |
On 29.05.2017 13:33, Peter Zijlstra wrote: > On Mon, May 29, 2017 at 12:24:53PM +0300, Alexey Budankov wrote: >> On 29.05.2017 10:45, Peter Zijlstra wrote: >>> On Sat, May 27, 2017 at 02:19:51PM +0300, Alexey Budankov wrote: >>>> Solution: >>>> >>>> cpu indexed trees for perf_event_context::pinned_groups and >>>> perf_event_context::flexible_groups lists are introduced. Every tree node >>>> keeps a list of groups allocated for the same cpu. A tree references only >>>> groups located at the appropriate group list. The tree provides capability >>>> to iterate over groups allocated for a specific cpu only, what is exactly >>>> required by multiplexing timer interrupt handler. The handler runs per-cpu >>>> and enables/disables groups using group_sched_in()/group_sched_out() that >>>> call event_filter_match() function filtering out groups allocated for cpus >>>> different from the one executing the handler. Additionally for every >>>> filtered out group group_sched_out() updates tstamps values to the current >>>> interrupt time. This updating work is now done only once by >>>> update_context_time() called by ctx_sched_out() before cpu groups >>>> iteration. For this trick to work it is required that tstamps of filtered >>>> out events would point to perf_event_context::tstamp_data object instead >>>> of perf_event::tstamp_data ones, as it is initialized from an event >>>> allocation. tstamps references are switched by >>>> group_sched_in()/group_sched_out() every time a group is checked for its >>>> suitability for currently running cpu. When a thread enters some cpu on >>>> a context switch a long run through pinned and flexible groups is >>>> performed by perf_event_sched_in(, mux=0) with new parameter mux set to 0 >>>> and filtered out groups tstamps are switched to >>>> perf_event_context::tstamp_data object. Then a series of multiplexing >>>> interrupts happens and the handler rotates the flexible groups calling >>>> ctx_sched_out(,mux=1)/perf_event_sched_in(,mux=1) iterating over the cpu >>>> tree lists only and avoiding long runs through the complete group lists. >>>> This is where speedup comes from. Eventually when the thread leaves the cpu >>>> ctx_sched_out(,mux=0) is called restoring tstamps pointers to the events' >>>> perf_event::tstamp_data objects. >>> >>> This is unreadable.. Please use whitespace. >> >> Do you mean do NOT use whitespaces? Could you explain in more detail what >> you mean? > > No, add _more_ whitespace. Use things like paragraphs and such. Reading > a massive blob of text like that is painful. Also use full and complete > sentences. For example, the very first sentence: > > 'cpu indexed trees for perf_event_context::pinned_groups and > perf_event_context::flexible_groups lists are introduced.' > > feels incomplete and leaves one wondering what for etc.. And it only > gets worse. > Makes sense. Will do. Thanks. > >>> Yeah, this doesn't go into a changelog. Have you _ever_ seen a changelog >>> with such crud in? >>> >> >> I have not. Could you please advise how to format this information to be >> suitable for changelog, if it's required at all? > > Don't include it. It should be fairly obvious from the diff itself what > changed after all. > Ok. Clear.
[toc] | [prev] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2017-05-31 23:40 +0200 |
| Subject | Re: [PATCH v2]: perf/core: addressing 4x slowdown during per-process, profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tNjvr-20L-15@gated-at.bofh.it> |
| In reply to | #1651863 |
On Sat, May 27, 2017 at 4:19 AM, Alexey Budankov
<alexey.budankov@linux.intel.com> wrote:
> Motivation:
>
> The issue manifests like 4x slowdown when profiling single thread STREAM
> benchmark on Intel Xeon Phi running RHEL7.2 (Intel MPSS distribution).
> Perf profiling is done in per-process mode and involves about 30 core
> events. In case the benchmark is OpenMP based and runs under profiling in
> 272 threads the overhead becomes even more dramatic: 512.144s against
> 36.192s (with this patch).
How long does it take without any perf monitoring? Could you provide
more details about the benchmark? how many CPUs are being monitored?
SNIP
> different from the one executing the handler. Additionally for every
> filtered out group group_sched_out() updates tstamps values to the current
> interrupt time. This updating work is now done only once by
> update_context_time() called by ctx_sched_out() before cpu groups
> iteration.
I don't see this. e.g.:
in your patch task_ctx_sched_out calls ctx_sched_out with mux == 0,
that path does the exact same thing as before your patch.
I understand why you want to move the event's times to a different
structure and keep a pointer in the event, but I don't see that you
are avoiding the update of the times of unscheduled events.
>
> static u64 perf_event_time(struct perf_event *event)
> @@ -1424,16 +1428,16 @@ static void update_event_times(struct perf_event
> *event)
> else if (ctx->is_active)
> run_end = ctx->time;
> else
> - run_end = event->tstamp_stopped;
> + run_end = event->tstamp->stopped;
>
> - event->total_time_enabled = run_end - event->tstamp_enabled;
> + event->total_time_enabled = run_end - event->tstamp->enabled;
>
> if (event->state == PERF_EVENT_STATE_INACTIVE)
> - run_end = event->tstamp_stopped;
> + run_end = event->tstamp->stopped;
> else
> run_end = perf_event_time(event);
>
> - event->total_time_running = run_end - event->tstamp_running;
> + event->total_time_running = run_end - event->tstamp->running;
FWICT, this is run for ALL events in context with matching CPU.
SNIP
> }
> @@ -3051,9 +3277,9 @@ void __perf_event_task_sched_out(struct task_struct
> *task,
> * Called with IRQs disabled
> */
> static void cpu_ctx_sched_out(struct perf_cpu_context *cpuctx,
> - enum event_type_t event_type)
> + enum event_type_t event_type, int mux)
> {
> - ctx_sched_out(&cpuctx->ctx, cpuctx, event_type);
> + ctx_sched_out(&cpuctx->ctx, cpuctx, event_type, mux);
> }
>
> static void
> @@ -3061,29 +3287,8 @@ ctx_pinned_sched_in(struct perf_event_context *ctx,
> struct perf_cpu_context *cpuctx)
> {
> struct perf_event *event;
> -
> - list_for_each_entry(event, &ctx->pinned_groups, group_entry) {
> - if (event->state <= PERF_EVENT_STATE_OFF)
> - continue;
> - if (!event_filter_match(event))
> - continue;
Could we remove or simplify the tests in event_filter_match since the
rb-tree filters by cpu?
> -
> - /* may need to reset tstamp_enabled */
> - if (is_cgroup_event(event))
> - perf_cgroup_mark_enabled(event, ctx);
> -
> - if (group_can_go_on(event, cpuctx, 1))
> - group_sched_in(event, cpuctx, ctx);
> -
> - /*
> - * If this pinned group hasn't been scheduled,
> - * put it in error state.
> - */
> - if (event->state == PERF_EVENT_STATE_INACTIVE) {
> - update_group_times(event);
> - event->state = PERF_EVENT_STATE_ERROR;
> - }
> - }
> + list_for_each_entry(event, &ctx->pinned_groups, group_entry)
> + ctx_sched_in_pinned_group(ctx, cpuctx, event);
> }
>
> static void
> @@ -3092,37 +3297,19 @@ ctx_flexible_sched_in(struct perf_event_context
> *ctx,
> {
> struct perf_event *event;
> int can_add_hw = 1;
> -
> - list_for_each_entry(event, &ctx->flexible_groups, group_entry) {
> - /* Ignore events in OFF or ERROR state */
> - if (event->state <= PERF_EVENT_STATE_OFF)
> - continue;
> - /*
> - * Listen to the 'cpu' scheduling filter constraint
> - * of events:
> - */
> - if (!event_filter_match(event))
> - continue;
same as before.
[toc] | [prev] | [next] | [standalone]
| From | Alexander Shishkin <alexander.shishkin@linux.intel.com> |
|---|---|
| Date | 2017-05-29 14:10 +0200 |
| Subject | Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMrEK-865-25@gated-at.bofh.it> |
| In reply to | #1651699 |
Alexey Budankov <alexey.budankov@linux.intel.com> writes:
Here (above the function) you could include a comment describing what
happens when this is called, locking considerations, etc.
> +static int
> +perf_cpu_tree_insert(struct rb_root *tree, struct perf_event *event)
> +{
> + struct rb_node **node;
> + struct rb_node *parent;
> +
> + if (!tree || !event)
> + return 0;
I don't think this should be happening, should it? And either way you
probably don't want to return 0 here, unless you're using !0 for
success.
> +
> + node = &tree->rb_node;
> + parent = *node;
> +
> + while (*node) {
> + struct perf_event *node_event = container_of(*node,
> + struct perf_event, group_node);
> +
> + parent = *node;
> +
> + if (event->cpu < node_event->cpu) {
> + node = &((*node)->rb_left);
this would be the same as node = &parent->rb_left, right?
> + } else if (event->cpu > node_event->cpu) {
> + node = &((*node)->rb_right);
> + } else {
> + list_add_tail(&event->group_list_entry,
> + &node_event->group_list);
So why is this better than simply having per-cpu event lists plus one
for per-thread events?
Also,
> + return 2;
2?
> + }
> + }
> +
> + list_add_tail(&event->group_list_entry, &event->group_list);
> +
> + rb_link_node(&event->group_node, parent, node);
> + rb_insert_color(&event->group_node, tree);
> +
> + return 1;
Oh, you are using !0 for success. I guess it's a good thing you're not
actually checking its return code at the call site.
Regards,
--
Alex
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 15:50 +0200 |
| Message-ID | <tMtdv-A4-13@gated-at.bofh.it> |
| In reply to | #1652470 |
On 29.05.2017 15:03, Alexander Shishkin wrote:
> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>
> Here (above the function) you could include a comment describing what
> happens when this is called, locking considerations, etc.
I can put the short description from the initial thread message here.
Would it be sufficient?
>
>> +static int
>> +perf_cpu_tree_insert(struct rb_root *tree, struct perf_event *event)
>> +{
>> + struct rb_node **node;
>> + struct rb_node *parent;
>> +
>> + if (!tree || !event)
>> + return 0;
>
> I don't think this should be happening, should it? And either way you
> probably don't want to return 0 here, unless you're using !0 for
> success.
As you might notice already, currently return codes of the tree API are
not checked all other the implementation. I suggest replacing that int
error code by void and simplify the stuff.
>
>> +
>> + node = &tree->rb_node;
>> + parent = *node;
>> +
>> + while (*node) {
>> + struct perf_event *node_event = container_of(*node,
>> + struct perf_event, group_node);
>> +
>> + parent = *node;
>> +
>> + if (event->cpu < node_event->cpu) {
>> + node = &((*node)->rb_left);
>
> this would be the same as node = &parent->rb_left, right?
Please ask more. node is the leaf node and parent is the parent of the
node at the end of cycle. We need the both to insert a new node into a
tree.
>
>> + } else if (event->cpu > node_event->cpu) {
>> + node = &((*node)->rb_right);
>> + } else {
>> + list_add_tail(&event->group_list_entry,
>> + &node_event->group_list);
>
> So why is this better than simply having per-cpu event lists plus one
> for per-thread events?
Good question. Choice of data structure and layout depends on the
operations applied to the data so keeping groups as a tree simplifies
and improves the implementation in terms of scalability and performance.
Please ask more if any.
>
> Also,
>
>> + return 2;
>
> 2?
Answered above.
>
>> + }
>> + }
>> +
>> + list_add_tail(&event->group_list_entry, &event->group_list);
>> +
>> + rb_link_node(&event->group_node, parent, node);
>> + rb_insert_color(&event->group_node, tree);
>> +
>> + return 1;
>
> Oh, you are using !0 for success. I guess it's a good thing you're not
> actually checking its return code at the call site.
Answered above.
>
> Regards,
> --
> Alex
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 17:30 +0200 |
| Message-ID | <tMuMi-1KX-31@gated-at.bofh.it> |
| In reply to | #1652537 |
On Mon, May 29, 2017 at 04:43:09PM +0300, Alexey Budankov wrote:
> On 29.05.2017 15:03, Alexander Shishkin wrote:
> > Alexey Budankov <alexey.budankov@linux.intel.com> writes:
> > > + } else if (event->cpu > node_event->cpu) {
> > > + node = &((*node)->rb_right);
> > > + } else {
> > > + list_add_tail(&event->group_list_entry,
> > > + &node_event->group_list);
> >
> > So why is this better than simply having per-cpu event lists plus one
> > for per-thread events?
>
> Good question. Choice of data structure and layout depends on the operations
> applied to the data so keeping groups as a tree simplifies and improves the
> implementation in terms of scalability and performance. Please ask more if
> any.
Since these lists are per context, and each task can have a context,
you'd end up with per-task-per-cpu memory, which is something we'd like
to avoid (some archs have very limited per-cpu memory space etc..).
Also, we'd like to have that tree for other reasons, like for instance
that heterogeneous PMU crud ARM has. Also, with a tree we can easier do
time based round-robin scheduling,
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-29 17:30 +0200 |
| Message-ID | <tMuMi-1KX-35@gated-at.bofh.it> |
| In reply to | #1652590 |
On Mon, May 29, 2017 at 05:22:54PM +0200, Peter Zijlstra wrote:
> On Mon, May 29, 2017 at 04:43:09PM +0300, Alexey Budankov wrote:
> > On 29.05.2017 15:03, Alexander Shishkin wrote:
> > > Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>
> > > > + } else if (event->cpu > node_event->cpu) {
> > > > + node = &((*node)->rb_right);
> > > > + } else {
> > > > + list_add_tail(&event->group_list_entry,
> > > > + &node_event->group_list);
> > >
> > > So why is this better than simply having per-cpu event lists plus one
> > > for per-thread events?
> >
> > Good question. Choice of data structure and layout depends on the operations
> > applied to the data so keeping groups as a tree simplifies and improves the
> > implementation in terms of scalability and performance. Please ask more if
> > any.
>
> Since these lists are per context, and each task can have a context,
> you'd end up with per-task-per-cpu memory, which is something we'd like
> to avoid (some archs have very limited per-cpu memory space etc..).
>
> Also, we'd like to have that tree for other reasons, like for instance
> that heterogeneous PMU crud ARM has. Also, with a tree we can easier do
> time based round-robin scheduling,
>
Oh and in general multi-PMU stuff, aside from hetero PMU becomes much
easier.
[toc] | [prev] | [next] | [standalone]
| From | Alexey Budankov <alexey.budankov@linux.intel.com> |
|---|---|
| Date | 2017-05-29 18:50 +0200 |
| Message-ID | <tMw1J-2zI-21@gated-at.bofh.it> |
| In reply to | #1652592 |
On 29.05.2017 18:29, Peter Zijlstra wrote:
> On Mon, May 29, 2017 at 05:22:54PM +0200, Peter Zijlstra wrote:
>> On Mon, May 29, 2017 at 04:43:09PM +0300, Alexey Budankov wrote:
>>> On 29.05.2017 15:03, Alexander Shishkin wrote:
>>>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>
>>>>> + } else if (event->cpu > node_event->cpu) {
>>>>> + node = &((*node)->rb_right);
>>>>> + } else {
>>>>> + list_add_tail(&event->group_list_entry,
>>>>> + &node_event->group_list);
>>>>
>>>> So why is this better than simply having per-cpu event lists plus one
>>>> for per-thread events?
>>>
>>> Good question. Choice of data structure and layout depends on the operations
>>> applied to the data so keeping groups as a tree simplifies and improves the
>>> implementation in terms of scalability and performance. Please ask more if
>>> any.
>>
>> Since these lists are per context, and each task can have a context,
>> you'd end up with per-task-per-cpu memory, which is something we'd like
>> to avoid (some archs have very limited per-cpu memory space etc..).
Aw, yeah. Memory consumption does matter in the kernel space.
>>
>> Also, we'd like to have that tree for other reasons, like for instance
>> that heterogeneous PMU crud ARM has. Also, with a tree we can easier do
>> time based round-robin scheduling,
>>
>
> Oh and in general multi-PMU stuff, aside from hetero PMU becomes much
> easier.
>
[toc] | [prev] | [next] | [standalone]
| From | Alexander Shishkin <alexander.shishkin@linux.intel.com> |
|---|---|
| Date | 2017-05-30 10:30 +0200 |
| Subject | Re: [PATCH]: perf/core: addressing 4x slowdown during per-process profiling of STREAM benchmark on Intel Xeon Phi |
| Message-ID | <tMKHo-4SA-21@gated-at.bofh.it> |
| In reply to | #1652537 |
Alexey Budankov <alexey.budankov@linux.intel.com> writes:
> On 29.05.2017 15:03, Alexander Shishkin wrote:
>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>
>> Here (above the function) you could include a comment describing what
>> happens when this is called, locking considerations, etc.
>
> I can put the short description from the initial thread message here.
> Would it be sufficient?
Sure, this is where API descriptions fit better than in commit messages.
>
>>
>>> +static int
>>> +perf_cpu_tree_insert(struct rb_root *tree, struct perf_event *event)
>>> +{
>>> + struct rb_node **node;
>>> + struct rb_node *parent;
>>> +
>>> + if (!tree || !event)
>>> + return 0;
>>
>> I don't think this should be happening, should it? And either way you
>> probably don't want to return 0 here, unless you're using !0 for
>> success.
>
> As you might notice already, currently return codes of the tree API are
> not checked all other the implementation. I suggest replacing that int
> error code by void and simplify the stuff.
Your call. But I'd still either drop the redundant checks or wrap them
in WARN_ON_ONCE().
>
>>
>>> +
>>> + node = &tree->rb_node;
>>> + parent = *node;
>>> +
>>> + while (*node) {
>>> + struct perf_event *node_event = container_of(*node,
>>> + struct perf_event, group_node);
>>> +
>>> + parent = *node;
>>> +
>>> + if (event->cpu < node_event->cpu) {
>>> + node = &((*node)->rb_left);
>>
>> this would be the same as node = &parent->rb_left, right?
>
> Please ask more.
Side note: between commit message, comments and the actual code, in an
ideal situation one doesn't have to 'ask' anything, because everything
is already clear. Not the case here.
> node is the leaf node and parent is the parent of the
> node at the end of cycle. We need the both to insert a new node into a
> tree.
Not sure I understand. You'd still have both.
>
>>
>>> + } else if (event->cpu > node_event->cpu) {
>>> + node = &((*node)->rb_right);
>>> + } else {
>>> + list_add_tail(&event->group_list_entry,
>>> + &node_event->group_list);
>>
>> So why is this better than simply having per-cpu event lists plus one
>> for per-thread events?
>
> Good question. Choice of data structure and layout depends on the
> operations applied to the data so keeping groups as a tree simplifies
> and improves the implementation in terms of scalability and performance.
> Please ask more if any.
Please be more specific on how scalability and performance are
improved. In general, try to avoid vagues statements like "this is
better for performance".
Thanks,
--
Alex
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web