Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1453560 > unrolled thread
| Started by | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| First post | 2016-08-02 05:10 +0200 |
| Last post | 2016-08-02 11:20 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] perf/core: set cgroup for cpu contexts for new cgroup events David Carrillo-Cisneros <davidcc@google.com> - 2016-08-02 05:10 +0200
Re: [PATCH] perf/core: set cgroup for cpu contexts for new cgroup events kbuild test robot <lkp@intel.com> - 2016-08-02 05:40 +0200
Re: [PATCH] perf/core: set cgroup for cpu contexts for new cgroup events David Carrillo-Cisneros <davidcc@google.com> - 2016-08-02 11:20 +0200
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2016-08-02 05:10 +0200 |
| Subject | [PATCH] perf/core: set cgroup for cpu contexts for new cgroup events |
| Message-ID | <s1yfD-74R-11@gated-at.bofh.it> |
There is an optimization in perf_cgroup_sched_{in,out} that skips the
switch of cgroup events if the old and new cgroups in a task context
switch are the same. This optimization interacts with the current code
in two ways that cause a cpu context's cgroup (cpuctx->cgrp) to be NULL
despite having a cgroup event that matches the current task. These are:
1. On creation of the first cgroup event in a CPU: In current code,
cpuctx->cpu is only set in perf_cgroup_sched_in, but due to the
aforesaid optimization, perf_cgroup_sched_in will run until the next
cgroup switch in that cpu. This may happen late or never happen,
depending on system's number of cgroups, cpu load, etc.
2. On deletion of the last cgroup event in a cpuctx: In list_del_event,
cpuctx->cgrp is set NULL. Any new cgroup event will not be sched in
because cpuctx->cgrp == NULL until a cgroup switch occurs and
perf_cgroup_sched_in is executed (updating cpuctx->cgrp).
This patch fixes both problems by setting cpuctx->cgrp in list_add_event,
mirroring what list_del_event does when removing a cgroup event from CPU
context, as introduced in:
commit 68cacd29167b ("perf_events: Fix stale ->cgrp pointer in update_cgrp_time_from_cpuctx()")
With this patch, cpuctx->cgrp is always set/clear when installing/removing
the first/last cgroup event in/from the cpu context. Having cpuctx->cgrp
correctly set since the event is installed in the context allows
event_filter_match to work correctly while scheduling in and out events
without relying on a cgroup switch that may occur late (if ever).
The problem is easy to observe in a machine with only one cgroup:
$ perf stat -e cycles -I 1000 -C 0 -G /
# time counts unit events
1.000161699 <not counted> cycles /
2.000355591 <not counted> cycles /
3.000565154 <not counted> cycles /
4.000951350 <not counted> cycles /
After the fix, the output is as expected:
$ perf stat -e cycles -I 1000 -a -G /
# time counts unit events
1.004699159 627342882 cycles /
2.007397156 615272690 cycles /
3.010019057 616726074 cycles /
This patch also do minor style edit into list_del_event.
Rebased at peterz/queue/perf/core.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
kernel/events/core.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9345028..1efa89d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1392,6 +1392,8 @@ ctx_group_list(struct perf_event *event, struct perf_event_context *ctx)
static void
list_add_event(struct perf_event *event, struct perf_event_context *ctx)
{
+ struct perf_cpu_context *cpuctx;
+
lockdep_assert_held(&ctx->lock);
WARN_ON_ONCE(event->attach_state & PERF_ATTACH_CONTEXT);
@@ -1412,8 +1414,21 @@ list_add_event(struct perf_event *event, struct perf_event_context *ctx)
list_add_tail(&event->group_entry, list);
}
- if (is_cgroup_event(event))
- ctx->nr_cgroups++;
+ if (is_cgroup_event(event)) {
+ /*
+ * If there are no more cgroup events, set cgrp in context
+ * so event_filter_match works.
+ */
+ if (!ctx->nr_cgroups++) {
+ /*
+ * Because cgroup events are always per-cpu events,
+ * this will always be called from the right CPU.
+ */
+ cpuctx = __get_cpu_context(ctx);
+ cpuctx->cgrp = event->cgrp;
+ }
+ }
+
list_add_rcu(&event->event_entry, &ctx->event_list);
ctx->nr_events++;
@@ -1595,18 +1610,18 @@ list_del_event(struct perf_event *event, struct perf_event_context *ctx)
event->attach_state &= ~PERF_ATTACH_CONTEXT;
if (is_cgroup_event(event)) {
- ctx->nr_cgroups--;
- /*
- * Because cgroup events are always per-cpu events, this will
- * always be called from the right CPU.
- */
- cpuctx = __get_cpu_context(ctx);
/*
* If there are no more cgroup events then clear cgrp to avoid
* stale pointer in update_cgrp_time_from_cpuctx().
*/
- if (!ctx->nr_cgroups)
+ if (!--ctx->nr_cgroups) {
+ /*
+ * Because cgroup events are always per-cpu events,
+ * this will always be called from the right CPU.
+ */
+ cpuctx = __get_cpu_context(ctx);
cpuctx->cgrp = NULL;
+ }
}
ctx->nr_events--;
--
2.8.0.rc3.226.g39d4020
[toc] | [next] | [standalone]
| From | kbuild test robot <lkp@intel.com> |
|---|---|
| Date | 2016-08-02 05:40 +0200 |
| Subject | Re: [PATCH] perf/core: set cgroup for cpu contexts for new cgroup events |
| Message-ID | <s1yIG-7go-11@gated-at.bofh.it> |
| In reply to | #1453560 |
[Multipart message — attachments visible in raw view] — view raw
Hi David,
[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.7 next-20160801]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/David-Carrillo-Cisneros/perf-core-set-cgroup-for-cpu-contexts-for-new-cgroup-events/20160802-110924
config: x86_64-randconfig-x012-201631 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
kernel/events/core.c: In function 'list_add_event':
>> kernel/events/core.c:1428:24: error: 'struct perf_event' has no member named 'cgrp'
cpuctx->cgrp = event->cgrp;
^~
vim +1428 kernel/events/core.c
1422 if (!ctx->nr_cgroups++) {
1423 /*
1424 * Because cgroup events are always per-cpu events,
1425 * this will always be called from the right CPU.
1426 */
1427 cpuctx = __get_cpu_context(ctx);
> 1428 cpuctx->cgrp = event->cgrp;
1429 }
1430 }
1431
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2016-08-02 11:20 +0200 |
| Message-ID | <s1E1I-2tq-7@gated-at.bofh.it> |
| In reply to | #1453570 |
Sending new version with build error fixed in another thread.
On Mon, Aug 1, 2016 at 8:30 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi David,
>
> [auto build test ERROR on tip/perf/core]
> [also build test ERROR on v4.7 next-20160801]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/David-Carrillo-Cisneros/perf-core-set-cgroup-for-cpu-contexts-for-new-cgroup-events/20160802-110924
> config: x86_64-randconfig-x012-201631 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
> kernel/events/core.c: In function 'list_add_event':
>>> kernel/events/core.c:1428:24: error: 'struct perf_event' has no member named 'cgrp'
> cpuctx->cgrp = event->cgrp;
> ^~
>
> vim +1428 kernel/events/core.c
>
> 1422 if (!ctx->nr_cgroups++) {
> 1423 /*
> 1424 * Because cgroup events are always per-cpu events,
> 1425 * this will always be called from the right CPU.
> 1426 */
> 1427 cpuctx = __get_cpu_context(ctx);
>> 1428 cpuctx->cgrp = event->cgrp;
> 1429 }
> 1430 }
> 1431
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web