Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1562097 > unrolled thread
| Started by | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| First post | 2017-01-18 20:30 +0100 |
| Last post | 2017-01-30 13:00 +0100 |
| Articles | 8 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/2] Optimize cgroup ctx switch and remove cpuctx->unique_pmu David Carrillo-Cisneros <davidcc@google.com> - 2017-01-18 20:30 +0100
[PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu David Carrillo-Cisneros <davidcc@google.com> - 2017-01-18 20:30 +0100
Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu Peter Zijlstra <peterz@infradead.org> - 2017-01-20 13:30 +0100
Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu Mark Rutland <mark.rutland@arm.com> - 2017-01-20 15:30 +0100
Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu David Carrillo-Cisneros <davidcc@google.com> - 2017-01-20 22:50 +0100
[tip:perf/core] perf/core: Remove perf_cpu_context::unique_pmu tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-01-30 13:00 +0100
[PATCH v2 1/2] perf/core: Make cgroup switch visit only cpuctxs with cgroup events David Carrillo-Cisneros <davidcc@google.com> - 2017-01-18 20:30 +0100
[tip:perf/core] perf/core: Make cgroup switch visit only cpuctxs with cgroup events tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-01-30 13:00 +0100
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2017-01-18 20:30 +0100 |
| Subject | [PATCH v2 0/2] Optimize cgroup ctx switch and remove cpuctx->unique_pmu |
| Message-ID | <t145H-86O-7@gated-at.bofh.it> |
v2: Rename local variable, as suggested by Mark Rutland and
add his Acked-by.
First patch is a resend of ("perf/core: Make cgroup switch visit only
cpuctxs with cgroup events") with updated change log.
Second patch cleans up the now unused cpuctx->unique_pmu.
David Carrillo-Cisneros (2):
perf/core: Make cgroup switch visit only cpuctxs with cgroup events
perf/core: Remove perf_cpu_context::unique_pmu
include/linux/perf_event.h | 2 +-
kernel/events/core.c | 129 ++++++++++++++++-----------------------------
2 files changed, 47 insertions(+), 84 deletions(-)
--
2.11.0.483.g087da7b7c-goog
[toc] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2017-01-18 20:30 +0100 |
| Subject | [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu |
| Message-ID | <t145I-86O-33@gated-at.bofh.it> |
| In reply to | #1562097 |
cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
with shared pmus in order to avoid visiting the same cpuctx more than once
in a for_each_pmu loop.
cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in
hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
The change above, together with the previous patch in this series, removed
the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
include/linux/perf_event.h | 1 -
kernel/events/core.c | 31 +------------------------------
2 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 10d223572855..4f8a1f681aac 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -785,7 +785,6 @@ struct perf_cpu_context {
ktime_t hrtimer_interval;
unsigned int hrtimer_active;
- struct pmu *unique_pmu;
#ifdef CONFIG_CGROUP_PERF
struct perf_cgroup *cgrp;
struct list_head cgrp_cpuctx_entry;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5f5b760bc25b..65b706d25422 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2883,7 +2883,7 @@ static void perf_pmu_sched_task(struct task_struct *prev,
return;
list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
- pmu = cpuctx->unique_pmu; /* software PMUs will not have sched_task */
+ pmu = cpuctx->ctx.pmu; /* software PMUs will not have sched_task */
if (WARN_ON_ONCE(!pmu->sched_task))
continue;
@@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
return NULL;
}
-static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
- struct perf_cpu_context *cpuctx;
-
- cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-
- if (cpuctx->unique_pmu == old_pmu)
- cpuctx->unique_pmu = pmu;
- }
-}
-
static void free_pmu_context(struct pmu *pmu)
{
- struct pmu *i;
-
mutex_lock(&pmus_lock);
- /*
- * Like a real lame refcount.
- */
- list_for_each_entry(i, &pmus, entry) {
- if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
- update_pmu_context(i, pmu);
- goto out;
- }
- }
-
free_percpu(pmu->pmu_cpu_context);
-out:
mutex_unlock(&pmus_lock);
}
@@ -8806,8 +8779,6 @@ int perf_pmu_register(struct pmu *pmu, const char *name, int type)
cpuctx->ctx.pmu = pmu;
__perf_mux_hrtimer_init(cpuctx, cpu);
-
- cpuctx->unique_pmu = pmu;
}
got_cpu_context:
--
2.11.0.483.g087da7b7c-goog
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-01-20 13:30 +0100 |
| Subject | Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu |
| Message-ID | <t1Gun-7bo-29@gated-at.bofh.it> |
| In reply to | #1562103 |
On Wed, Jan 18, 2017 at 11:24:54AM -0800, David Carrillo-Cisneros wrote:
> cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
> with shared pmus in order to avoid visiting the same cpuctx more than once
> in a for_each_pmu loop.
>
> cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
> have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in
> hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
>
> The change above, together with the previous patch in this series, removed
> the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
>
> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> @@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
> return NULL;
> }
>
> -static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
> -{
> - int cpu;
> -
> - for_each_possible_cpu(cpu) {
> - struct perf_cpu_context *cpuctx;
> -
> - cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
> -
> - if (cpuctx->unique_pmu == old_pmu)
> - cpuctx->unique_pmu = pmu;
> - }
> -}
> -
> static void free_pmu_context(struct pmu *pmu)
> {
> - struct pmu *i;
> -
> mutex_lock(&pmus_lock);
> - /*
> - * Like a real lame refcount.
> - */
> - list_for_each_entry(i, &pmus, entry) {
> - if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
> - update_pmu_context(i, pmu);
> - goto out;
> - }
> - }
> -
> free_percpu(pmu->pmu_cpu_context);
> -out:
> mutex_unlock(&pmus_lock);
> }
This very much relies on us never calling perf_pmu_unregister() on the
software PMUs afaict. A condition not mention in the Changelog.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-20 15:30 +0100 |
| Subject | Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu |
| Message-ID | <t1Imv-8mw-51@gated-at.bofh.it> |
| In reply to | #1563536 |
On Fri, Jan 20, 2017 at 10:20:33AM +0100, Peter Zijlstra wrote:
> On Wed, Jan 18, 2017 at 11:24:54AM -0800, David Carrillo-Cisneros wrote:
> > cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
> > with shared pmus in order to avoid visiting the same cpuctx more than once
> > in a for_each_pmu loop.
> >
> > cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
> > have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in
> > hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
> >
> > The change above, together with the previous patch in this series, removed
> > the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
> >
> > Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> > @@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
> > return NULL;
> > }
> >
> > -static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
> > -{
> > - int cpu;
> > -
> > - for_each_possible_cpu(cpu) {
> > - struct perf_cpu_context *cpuctx;
> > -
> > - cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
> > -
> > - if (cpuctx->unique_pmu == old_pmu)
> > - cpuctx->unique_pmu = pmu;
> > - }
> > -}
> > -
> > static void free_pmu_context(struct pmu *pmu)
> > {
> > - struct pmu *i;
> > -
> > mutex_lock(&pmus_lock);
> > - /*
> > - * Like a real lame refcount.
> > - */
> > - list_for_each_entry(i, &pmus, entry) {
> > - if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
> > - update_pmu_context(i, pmu);
> > - goto out;
> > - }
> > - }
> > -
> > free_percpu(pmu->pmu_cpu_context);
> > -out:
> > mutex_unlock(&pmus_lock);
> > }
>
> This very much relies on us never calling perf_pmu_unregister() on the
> software PMUs afaict. A condition not mention in the Changelog.
Ah; I did not consider that that could leave perf_pmu_sched_task()
seeing a stale cpuctx->ctx.pmu.
That said, is this not already a problem elsewhere? We don't update
ctx->pmu in perf_pmu_unregister, so this would be a problem for any path
using ctx->pmu today (e.g. perf_event_context_sched_in()).
Thanks,
Mark.
[toc] | [prev] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2017-01-20 22:50 +0100 |
| Subject | Re: [PATCH v2 2/2] perf/core: Remove perf_cpu_context::unique_pmu |
| Message-ID | <t1Pei-48I-23@gated-at.bofh.it> |
| In reply to | #1563536 |
On Fri, Jan 20, 2017 at 1:20 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Jan 18, 2017 at 11:24:54AM -0800, David Carrillo-Cisneros wrote:
>> cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
>> with shared pmus in order to avoid visiting the same cpuctx more than once
>> in a for_each_pmu loop.
>>
>> cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
>> have only one pmu per cpuctx. Since perf_pmu_sched_task is only called in
>> hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
>>
>> The change above, together with the previous patch in this series, removed
>> the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
>>
>> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
>
>> @@ -8572,37 +8572,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
>> return NULL;
>> }
>>
>> -static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
>> -{
>> - int cpu;
>> -
>> - for_each_possible_cpu(cpu) {
>> - struct perf_cpu_context *cpuctx;
>> -
>> - cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
>> -
>> - if (cpuctx->unique_pmu == old_pmu)
>> - cpuctx->unique_pmu = pmu;
>> - }
>> -}
>> -
>> static void free_pmu_context(struct pmu *pmu)
>> {
>> - struct pmu *i;
>> -
>> mutex_lock(&pmus_lock);
>> - /*
>> - * Like a real lame refcount.
>> - */
>> - list_for_each_entry(i, &pmus, entry) {
>> - if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
>> - update_pmu_context(i, pmu);
>> - goto out;
>> - }
>> - }
>> -
>> free_percpu(pmu->pmu_cpu_context);
>> -out:
>> mutex_unlock(&pmus_lock);
>> }
>
> This very much relies on us never calling perf_pmu_unregister() on the
> software PMUs afaict. A condition not mention in the Changelog.
>
What's a good way to solve this? Update the Changelog or add code to
update ctx->pmu?
This issue would go away cleanly if we were to remove the context
sharing across pmu's. Would you support work in that direction?
Thanks,
David
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> |
|---|---|
| Date | 2017-01-30 13:00 +0100 |
| Subject | [tip:perf/core] perf/core: Remove perf_cpu_context::unique_pmu |
| Message-ID | <t5iMO-3x0-17@gated-at.bofh.it> |
| In reply to | #1562103 |
Commit-ID: 1fd7e416995401ec082fc0fe6090a223969beda5
Gitweb: http://git.kernel.org/tip/1fd7e416995401ec082fc0fe6090a223969beda5
Author: David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Wed, 18 Jan 2017 11:24:54 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 30 Jan 2017 12:01:14 +0100
perf/core: Remove perf_cpu_context::unique_pmu
cpuctx->unique_pmu was originally introduced as a way to identify cpuctxs
with shared pmus in order to avoid visiting the same cpuctx more than once
in a for_each_pmu loop.
cpuctx->unique_pmu == cpuctx->pmu in non-software task contexts since they
have only one pmu per cpuctx. Since perf_pmu_sched_task() is only called in
hw contexts, this patch replaces cpuctx->unique_pmu by cpuctx->pmu in it.
The change above, together with the previous patch in this series, removed
the remaining uses of cpuctx->unique_pmu, so we remove it altogether.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vikas Shivappa <vikas.shivappa@linux.intel.com>
Cc: Vince Weaver <vince@deater.net>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/20170118192454.58008-3-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/perf_event.h | 1 -
kernel/events/core.c | 31 +------------------------------
2 files changed, 1 insertion(+), 31 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index dfa7257..5c58e93 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -785,7 +785,6 @@ struct perf_cpu_context {
ktime_t hrtimer_interval;
unsigned int hrtimer_active;
- struct pmu *unique_pmu;
#ifdef CONFIG_CGROUP_PERF
struct perf_cgroup *cgrp;
struct list_head cgrp_cpuctx_entry;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 928a818..d92c7ad 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2932,7 +2932,7 @@ static void perf_pmu_sched_task(struct task_struct *prev,
return;
list_for_each_entry(cpuctx, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
- pmu = cpuctx->unique_pmu; /* software PMUs will not have sched_task */
+ pmu = cpuctx->ctx.pmu; /* software PMUs will not have sched_task */
if (WARN_ON_ONCE(!pmu->sched_task))
continue;
@@ -8636,37 +8636,10 @@ static struct perf_cpu_context __percpu *find_pmu_context(int ctxn)
return NULL;
}
-static void update_pmu_context(struct pmu *pmu, struct pmu *old_pmu)
-{
- int cpu;
-
- for_each_possible_cpu(cpu) {
- struct perf_cpu_context *cpuctx;
-
- cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-
- if (cpuctx->unique_pmu == old_pmu)
- cpuctx->unique_pmu = pmu;
- }
-}
-
static void free_pmu_context(struct pmu *pmu)
{
- struct pmu *i;
-
mutex_lock(&pmus_lock);
- /*
- * Like a real lame refcount.
- */
- list_for_each_entry(i, &pmus, entry) {
- if (i->pmu_cpu_context == pmu->pmu_cpu_context) {
- update_pmu_context(i, pmu);
- goto out;
- }
- }
-
free_percpu(pmu->pmu_cpu_context);
-out:
mutex_unlock(&pmus_lock);
}
@@ -8870,8 +8843,6 @@ skip_type:
cpuctx->ctx.pmu = pmu;
__perf_mux_hrtimer_init(cpuctx, cpu);
-
- cpuctx->unique_pmu = pmu;
}
got_cpu_context:
[toc] | [prev] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2017-01-18 20:30 +0100 |
| Subject | [PATCH v2 1/2] perf/core: Make cgroup switch visit only cpuctxs with cgroup events |
| Message-ID | <t145J-86O-49@gated-at.bofh.it> |
| In reply to | #1562097 |
v2: Rename local variable as suggested by Mark Rutland.
v1: This is a resend of v2 of ("perf/core: only check cpuctx with cgroup
events during cgroup switch") with updated changelog.
This patch follows from a conversation in CQM/CMT's last series about
speeding up the context switch for cgroup events:
https://patchwork.kernel.org/patch/9478617/
This is a low-hanging fruit optimization. It replaces the iteration over
the "pmus" list in cgroup switch by an iteration over a new list that
contains only cpuctxs with at least one cgroup event.
This is necessary because the number of pmus have increased over the years
e.g modern x86 server systems have well above 50 pmus.
The iteration over the full pmu list is unneccessary and can be costly in
heavy cache contention scenarios.
Below are some instrumentation measurements with 10, 50 and 90 percentiles
of the total cost of context switch before and after this optimization for
a simple array read/write microbenchark.
Contention
Level Nr events Before (us) After (us) Median
L2 L3 types (10%, 50%, 90%) (10%, 50%, 90% Speedup
--------------------------------------------------------------------------
Low Low 1 (1.72, 2.42, 5.85) (1.35, 1.64, 5.46) 29%
High Low 1 (2.08, 4.56, 19.8) (1720, 2.20, 13.7) 51%
High High 1 (2.86, 10.4, 12.7) (2.54, 4.32, 12.1) 58%
Low Low 2 (1.98, 3.20, 6.89) (1.68, 2.41, 8.89) 24%
High Low 2 (2.48, 5.28, 22.4) (2150, 3.69, 14.6) 30%
High High 2 (3.32, 8.09, 13.9) (2.80, 5.15, 13.7) 36%
where:
1 event type = cycles
2 event types = cycles,intel_cqm/llc_occupancy/
Contention L2 Low: workset < L2 cache size.
High: " >> L2 " " .
Contention L3 Low: workset of task on all sockets < L3 cache size.
High: " " " " " " >> L3 " " .
Median Speedup is (50%ile Before - 50%ile After) / 50%ile Before
Unsurprisingly, the benefits of this optimization decrease with the number
of cpuctxs with a cgroup events, yet, is never detrimental.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 98 +++++++++++++++++++++-------------------------
2 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 4741ecdb9817..10d223572855 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -788,6 +788,7 @@ struct perf_cpu_context {
struct pmu *unique_pmu;
#ifdef CONFIG_CGROUP_PERF
struct perf_cgroup *cgrp;
+ struct list_head cgrp_cpuctx_entry;
#endif
struct list_head sched_cb_entry;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ab15509fab8c..5f5b760bc25b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -678,6 +678,8 @@ perf_cgroup_set_timestamp(struct task_struct *task,
info->timestamp = ctx->timestamp;
}
+static DEFINE_PER_CPU(struct list_head, cgrp_cpuctx_list);
+
#define PERF_CGROUP_SWOUT 0x1 /* cgroup switch out every event */
#define PERF_CGROUP_SWIN 0x2 /* cgroup switch in events based on task */
@@ -690,61 +692,46 @@ perf_cgroup_set_timestamp(struct task_struct *task,
static void perf_cgroup_switch(struct task_struct *task, int mode)
{
struct perf_cpu_context *cpuctx;
- struct pmu *pmu;
+ struct list_head *list;
unsigned long flags;
/*
- * disable interrupts to avoid geting nr_cgroup
- * changes via __perf_event_disable(). Also
- * avoids preemption.
+ * Disable interrupts and preemption to avoid this CPU's
+ * cgrp_cpuctx_entry to change under us.
*/
local_irq_save(flags);
- /*
- * we reschedule only in the presence of cgroup
- * constrained events.
- */
+ list = this_cpu_ptr(&cgrp_cpuctx_list);
+ list_for_each_entry(cpuctx, list, cgrp_cpuctx_entry) {
+ WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
- list_for_each_entry_rcu(pmu, &pmus, entry) {
- cpuctx = this_cpu_ptr(pmu->pmu_cpu_context);
- if (cpuctx->unique_pmu != pmu)
- continue; /* ensure we process each cpuctx once */
+ perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+ perf_pmu_disable(cpuctx->ctx.pmu);
- /*
- * perf_cgroup_events says at least one
- * context on this CPU has cgroup events.
- *
- * ctx->nr_cgroups reports the number of cgroup
- * events for a context.
- */
- if (cpuctx->ctx.nr_cgroups > 0) {
- perf_ctx_lock(cpuctx, cpuctx->task_ctx);
- perf_pmu_disable(cpuctx->ctx.pmu);
-
- if (mode & PERF_CGROUP_SWOUT) {
- cpu_ctx_sched_out(cpuctx, EVENT_ALL);
- /*
- * must not be done before ctxswout due
- * to event_filter_match() in event_sched_out()
- */
- cpuctx->cgrp = NULL;
- }
+ if (mode & PERF_CGROUP_SWOUT) {
+ cpu_ctx_sched_out(cpuctx, EVENT_ALL);
+ /*
+ * must not be done before ctxswout due
+ * to event_filter_match() in event_sched_out()
+ */
+ cpuctx->cgrp = NULL;
+ }
- if (mode & PERF_CGROUP_SWIN) {
- WARN_ON_ONCE(cpuctx->cgrp);
- /*
- * set cgrp before ctxsw in to allow
- * event_filter_match() to not have to pass
- * task around
- * we pass the cpuctx->ctx to perf_cgroup_from_task()
- * because cgorup events are only per-cpu
- */
- cpuctx->cgrp = perf_cgroup_from_task(task, &cpuctx->ctx);
- cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
- }
- perf_pmu_enable(cpuctx->ctx.pmu);
- perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
+ if (mode & PERF_CGROUP_SWIN) {
+ WARN_ON_ONCE(cpuctx->cgrp);
+ /*
+ * set cgrp before ctxsw in to allow
+ * event_filter_match() to not have to pass
+ * task around
+ * we pass the cpuctx->ctx to perf_cgroup_from_task()
+ * because cgorup events are only per-cpu
+ */
+ cpuctx->cgrp = perf_cgroup_from_task(task,
+ &cpuctx->ctx);
+ cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
}
+ perf_pmu_enable(cpuctx->ctx.pmu);
+ perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
}
local_irq_restore(flags);
@@ -889,6 +876,7 @@ list_update_cgroup_event(struct perf_event *event,
struct perf_event_context *ctx, bool add)
{
struct perf_cpu_context *cpuctx;
+ struct list_head *cpuctx_entry;
if (!is_cgroup_event(event))
return;
@@ -902,15 +890,16 @@ list_update_cgroup_event(struct perf_event *event,
* this will always be called from the right CPU.
*/
cpuctx = __get_cpu_context(ctx);
-
- /*
- * cpuctx->cgrp is NULL until a cgroup event is sched in or
- * ctx->nr_cgroup == 0 .
- */
- if (add && perf_cgroup_from_task(current, ctx) == event->cgrp)
- cpuctx->cgrp = event->cgrp;
- else if (!add)
+ cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
+ /* cpuctx->cgrp is NULL unless a cgroup event is active in this CPU .*/
+ if (add) {
+ list_add(cpuctx_entry, this_cpu_ptr(&cgrp_cpuctx_list));
+ if (perf_cgroup_from_task(current, ctx) == event->cgrp)
+ cpuctx->cgrp = event->cgrp;
+ } else {
+ list_del(cpuctx_entry);
cpuctx->cgrp = NULL;
+ }
}
#else /* !CONFIG_CGROUP_PERF */
@@ -10595,6 +10584,9 @@ static void __init perf_event_init_all_cpus(void)
INIT_LIST_HEAD(&per_cpu(pmu_sb_events.list, cpu));
raw_spin_lock_init(&per_cpu(pmu_sb_events.lock, cpu));
+#ifdef CONFIG_CGROUP_PERF
+ INIT_LIST_HEAD(&per_cpu(cgrp_cpuctx_list, cpu));
+#endif
INIT_LIST_HEAD(&per_cpu(sched_cb_list, cpu));
}
}
--
2.11.0.483.g087da7b7c-goog
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> |
|---|---|
| Date | 2017-01-30 13:00 +0100 |
| Subject | [tip:perf/core] perf/core: Make cgroup switch visit only cpuctxs with cgroup events |
| Message-ID | <t5iMN-3x0-9@gated-at.bofh.it> |
| In reply to | #1562104 |
Commit-ID: 058fe1c0440e68a1ba3c2270ae43e9f0298b27d8
Gitweb: http://git.kernel.org/tip/058fe1c0440e68a1ba3c2270ae43e9f0298b27d8
Author: David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Wed, 18 Jan 2017 11:24:53 -0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 30 Jan 2017 12:01:13 +0100
perf/core: Make cgroup switch visit only cpuctxs with cgroup events
This patch follows from a conversation in CQM/CMT's last series about
speeding up the context switch for cgroup events:
https://patchwork.kernel.org/patch/9478617/
This is a low-hanging fruit optimization. It replaces the iteration over
the "pmus" list in cgroup switch by an iteration over a new list that
contains only cpuctxs with at least one cgroup event.
This is necessary because the number of PMUs have increased over the years
e.g modern x86 server systems have well above 50 PMUs.
The iteration over the full PMU list is unneccessary and can be costly in
heavy cache contention scenarios.
Below are some instrumentation measurements with 10, 50 and 90 percentiles
of the total cost of context switch before and after this optimization for
a simple array read/write microbenchark.
Contention
Level Nr events Before (us) After (us) Median
L2 L3 types (10%, 50%, 90%) (10%, 50%, 90% Speedup
--------------------------------------------------------------------------
Low Low 1 (1.72, 2.42, 5.85) (1.35, 1.64, 5.46) 29%
High Low 1 (2.08, 4.56, 19.8) (1720, 2.20, 13.7) 51%
High High 1 (2.86, 10.4, 12.7) (2.54, 4.32, 12.1) 58%
Low Low 2 (1.98, 3.20, 6.89) (1.68, 2.41, 8.89) 24%
High Low 2 (2.48, 5.28, 22.4) (2150, 3.69, 14.6) 30%
High High 2 (3.32, 8.09, 13.9) (2.80, 5.15, 13.7) 36%
where:
1 event type = cycles
2 event types = cycles,intel_cqm/llc_occupancy/
Contention L2 Low: workset < L2 cache size.
High: " >> L2 " " .
Contention L3 Low: workset of task on all sockets < L3 cache size.
High: " " " " " " >> L3 " " .
Median Speedup is (50%ile Before - 50%ile After) / 50%ile Before
Unsurprisingly, the benefits of this optimization decrease with the number
of cpuctxs with a cgroup events, yet, is never detrimental.
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vikas Shivappa <vikas.shivappa@linux.intel.com>
Cc: Vince Weaver <vince@deater.net>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/20170118192454.58008-2-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 98 +++++++++++++++++++++-------------------------
2 files changed, 46 insertions(+), 53 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 78ed810..dfa7257 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -788,6 +788,7 @@ struct perf_cpu_context {
struct pmu *unique_pmu;
#ifdef CONFIG_CGROUP_PERF
struct perf_cgroup *cgrp;
+ struct list_head cgrp_cpuctx_entry;
#endif
struct list_head sched_cb_entry;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e5aaa80..928a818 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -678,6 +678,8 @@ perf_cgroup_set_timestamp(struct task_struct *task,
info->timestamp = ctx->timestamp;
}
+static DEFINE_PER_CPU(struct list_head, cgrp_cpuctx_list);
+
#define PERF_CGROUP_SWOUT 0x1 /* cgroup switch out every event */
#define PERF_CGROUP_SWIN 0x2 /* cgroup switch in events based on task */
@@ -690,61 +692,46 @@ perf_cgroup_set_timestamp(struct task_struct *task,
static void perf_cgroup_switch(struct task_struct *task, int mode)
{
struct perf_cpu_context *cpuctx;
- struct pmu *pmu;
+ struct list_head *list;
unsigned long flags;
/*
- * disable interrupts to avoid geting nr_cgroup
- * changes via __perf_event_disable(). Also
- * avoids preemption.
+ * Disable interrupts and preemption to avoid this CPU's
+ * cgrp_cpuctx_entry to change under us.
*/
local_irq_save(flags);
- /*
- * we reschedule only in the presence of cgroup
- * constrained events.
- */
+ list = this_cpu_ptr(&cgrp_cpuctx_list);
+ list_for_each_entry(cpuctx, list, cgrp_cpuctx_entry) {
+ WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
- list_for_each_entry_rcu(pmu, &pmus, entry) {
- cpuctx = this_cpu_ptr(pmu->pmu_cpu_context);
- if (cpuctx->unique_pmu != pmu)
- continue; /* ensure we process each cpuctx once */
+ perf_ctx_lock(cpuctx, cpuctx->task_ctx);
+ perf_pmu_disable(cpuctx->ctx.pmu);
- /*
- * perf_cgroup_events says at least one
- * context on this CPU has cgroup events.
- *
- * ctx->nr_cgroups reports the number of cgroup
- * events for a context.
- */
- if (cpuctx->ctx.nr_cgroups > 0) {
- perf_ctx_lock(cpuctx, cpuctx->task_ctx);
- perf_pmu_disable(cpuctx->ctx.pmu);
-
- if (mode & PERF_CGROUP_SWOUT) {
- cpu_ctx_sched_out(cpuctx, EVENT_ALL);
- /*
- * must not be done before ctxswout due
- * to event_filter_match() in event_sched_out()
- */
- cpuctx->cgrp = NULL;
- }
+ if (mode & PERF_CGROUP_SWOUT) {
+ cpu_ctx_sched_out(cpuctx, EVENT_ALL);
+ /*
+ * must not be done before ctxswout due
+ * to event_filter_match() in event_sched_out()
+ */
+ cpuctx->cgrp = NULL;
+ }
- if (mode & PERF_CGROUP_SWIN) {
- WARN_ON_ONCE(cpuctx->cgrp);
- /*
- * set cgrp before ctxsw in to allow
- * event_filter_match() to not have to pass
- * task around
- * we pass the cpuctx->ctx to perf_cgroup_from_task()
- * because cgorup events are only per-cpu
- */
- cpuctx->cgrp = perf_cgroup_from_task(task, &cpuctx->ctx);
- cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
- }
- perf_pmu_enable(cpuctx->ctx.pmu);
- perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
+ if (mode & PERF_CGROUP_SWIN) {
+ WARN_ON_ONCE(cpuctx->cgrp);
+ /*
+ * set cgrp before ctxsw in to allow
+ * event_filter_match() to not have to pass
+ * task around
+ * we pass the cpuctx->ctx to perf_cgroup_from_task()
+ * because cgorup events are only per-cpu
+ */
+ cpuctx->cgrp = perf_cgroup_from_task(task,
+ &cpuctx->ctx);
+ cpu_ctx_sched_in(cpuctx, EVENT_ALL, task);
}
+ perf_pmu_enable(cpuctx->ctx.pmu);
+ perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
}
local_irq_restore(flags);
@@ -889,6 +876,7 @@ list_update_cgroup_event(struct perf_event *event,
struct perf_event_context *ctx, bool add)
{
struct perf_cpu_context *cpuctx;
+ struct list_head *cpuctx_entry;
if (!is_cgroup_event(event))
return;
@@ -902,15 +890,16 @@ list_update_cgroup_event(struct perf_event *event,
* this will always be called from the right CPU.
*/
cpuctx = __get_cpu_context(ctx);
-
- /*
- * cpuctx->cgrp is NULL until a cgroup event is sched in or
- * ctx->nr_cgroup == 0 .
- */
- if (add && perf_cgroup_from_task(current, ctx) == event->cgrp)
- cpuctx->cgrp = event->cgrp;
- else if (!add)
+ cpuctx_entry = &cpuctx->cgrp_cpuctx_entry;
+ /* cpuctx->cgrp is NULL unless a cgroup event is active in this CPU .*/
+ if (add) {
+ list_add(cpuctx_entry, this_cpu_ptr(&cgrp_cpuctx_list));
+ if (perf_cgroup_from_task(current, ctx) == event->cgrp)
+ cpuctx->cgrp = event->cgrp;
+ } else {
+ list_del(cpuctx_entry);
cpuctx->cgrp = NULL;
+ }
}
#else /* !CONFIG_CGROUP_PERF */
@@ -10709,6 +10698,9 @@ static void __init perf_event_init_all_cpus(void)
INIT_LIST_HEAD(&per_cpu(pmu_sb_events.list, cpu));
raw_spin_lock_init(&per_cpu(pmu_sb_events.lock, cpu));
+#ifdef CONFIG_CGROUP_PERF
+ INIT_LIST_HEAD(&per_cpu(cgrp_cpuctx_list, cpu));
+#endif
INIT_LIST_HEAD(&per_cpu(sched_cb_list, cpu));
}
}
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web