Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1464797 > unrolled thread
| Started by | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| First post | 2016-08-17 23:00 +0200 |
| Last post | 2016-08-18 13:50 +0200 |
| Articles | 9 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v3 0/4] remove unnecessary IPI reading uncore events David Carrillo-Cisneros <davidcc@google.com> - 2016-08-17 23:00 +0200
[PATCH v3 1/4] perf/core: check return value of perf_event_read IPI David Carrillo-Cisneros <davidcc@google.com> - 2016-08-17 23:00 +0200
[tip:perf/core] perf/core: Check return value of the perf_event_read() IPI tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2016-08-18 13:00 +0200
Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI Vegard Nossum <vegard.nossum@gmail.com> - 2016-08-21 14:20 +0200
Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI Jiri Olsa <jolsa@redhat.com> - 2016-08-22 09:20 +0200
Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI Jiri Olsa <jolsa@redhat.com> - 2016-08-22 10:30 +0200
Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI Jiri Olsa <jolsa@redhat.com> - 2016-08-22 12:40 +0200
[PATCH v3 3/4] perf/core: introduce PMU_EV_CAP_READ_ACTIVE_PKG David Carrillo-Cisneros <davidcc@google.com> - 2016-08-17 23:00 +0200
[tip:perf/core] perf/core: Introduce PMU_EV_CAP_READ_ACTIVE_PKG tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2016-08-18 13:50 +0200
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2016-08-17 23:00 +0200 |
| Subject | [PATCH v3 0/4] remove unnecessary IPI reading uncore events |
| Message-ID | <s7g6l-6kR-5@gated-at.bofh.it> |
This patch series adds a new flag to the struct perf_event (and a flag field to store it) to allow a PMU to tag a CPU or cgroup event as readable from any CPU in the same package and not just the CPU where the is currently active. This capability is used with uncore events to potentially avoid an unnecessary IPI when executing perf_event_read. A previous version of this change was introduced in the last Intel's CQM/CMT driver series (under review), but now we present it separately here since it is also useful for other uncore events. The next version of Intel CQM/CMT will add 3 new flags that use the pmu_event_flags field (added in patch 03 in this series). Patch 02 generalizes event->group_flags so that new flags can use it in a similar way that PERF_GROUP_SOFTWARE was used. Patches rebased at peterz/queue/perf/core Changes in v3: - Rebase to branch's tip. - Add warning to patch 01, in response to Ingo Molnar's comments. - Check for Active state in perf_event_read, as suggested by Nilay Vaish (already posted in v2's email thread). Changes in v2: - Change logic to use event->group_flags instead of individually testing sibling in perf_event_read. - Remove erroneous read of inactive events. David Carrillo-Cisneros (4): perf/core: check return value of perf_event_read IPI perf/core: generalize event->group_flags perf/core: introduce PMU_EV_CAP_READ_ACTIVE_PKG perf/x86: use PMUEF_READ_CPU_PKG in uncore events arch/x86/events/intel/rapl.c | 2 ++ arch/x86/events/intel/uncore.c | 2 ++ arch/x86/events/intel/uncore_snb.c | 2 ++ include/linux/perf_event.h | 21 +++++++++++++---- kernel/events/core.c | 47 ++++++++++++++++++++++++++++---------- 5 files changed, 57 insertions(+), 17 deletions(-) -- 2.8.0.rc3.226.g39d4020
[toc] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2016-08-17 23:00 +0200 |
| Subject | [PATCH v3 1/4] perf/core: check return value of perf_event_read IPI |
| Message-ID | <s7g6m-6kR-19@gated-at.bofh.it> |
| In reply to | #1464797 |
The call to smp_call_function_single in perf_event_read() may fail if
an invalid or not online CPU index is passed. Warn user if such bug is
present and return error.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
kernel/events/core.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 803481c..35afbfa 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3524,9 +3524,11 @@ static int perf_event_read(struct perf_event *event, bool group)
.group = group,
.ret = 0,
};
- smp_call_function_single(event->oncpu,
- __perf_event_read, &data, 1);
- ret = data.ret;
+ ret = smp_call_function_single(event->oncpu,
+ __perf_event_read, &data, 1);
+ /* event must have been read from an online CPU. */
+ WARN_ON_ONCE(ret);
+ ret = ret ? : data.ret;
} else if (event->state == PERF_EVENT_STATE_INACTIVE) {
struct perf_event_context *ctx = event->ctx;
unsigned long flags;
--
2.8.0.rc3.226.g39d4020
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> |
|---|---|
| Date | 2016-08-18 13:00 +0200 |
| Subject | [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI |
| Message-ID | <s7tdg-75c-27@gated-at.bofh.it> |
| In reply to | #1464798 |
Commit-ID: 71e7bc2bab77e64882c031c2af943c3256c1adb0
Gitweb: http://git.kernel.org/tip/71e7bc2bab77e64882c031c2af943c3256c1adb0
Author: David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Wed, 17 Aug 2016 13:55:04 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 10:35:52 +0200
perf/core: Check return value of the perf_event_read() IPI
The call to smp_call_function_single in perf_event_read() may fail if
an invalid or not online CPU index is passed. Warn user if such bug is
present and return error.
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.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: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1471467307-61171-2-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/events/core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index a5fc5c8..5650f53 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3549,9 +3549,10 @@ static int perf_event_read(struct perf_event *event, bool group)
.group = group,
.ret = 0,
};
- smp_call_function_single(event->oncpu,
- __perf_event_read, &data, 1);
- ret = data.ret;
+ ret = smp_call_function_single(event->oncpu, __perf_event_read, &data, 1);
+ /* The event must have been read from an online CPU: */
+ WARN_ON_ONCE(ret);
+ ret = ret ? : data.ret;
} else if (event->state == PERF_EVENT_STATE_INACTIVE) {
struct perf_event_context *ctx = event->ctx;
unsigned long flags;
[toc] | [prev] | [next] | [standalone]
| From | Vegard Nossum <vegard.nossum@gmail.com> |
|---|---|
| Date | 2016-08-21 14:20 +0200 |
| Subject | Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI |
| Message-ID | <s8zTk-d9-9@gated-at.bofh.it> |
| In reply to | #1465137 |
On 18 August 2016 at 12:52, tip-bot for David Carrillo-Cisneros
<tipbot@zytor.com> wrote:
> Commit-ID: 71e7bc2bab77e64882c031c2af943c3256c1adb0
> Gitweb: http://git.kernel.org/tip/71e7bc2bab77e64882c031c2af943c3256c1adb0
> Author: David Carrillo-Cisneros <davidcc@google.com>
> AuthorDate: Wed, 17 Aug 2016 13:55:04 -0700
> Committer: Ingo Molnar <mingo@kernel.org>
> CommitDate: Thu, 18 Aug 2016 10:35:52 +0200
>
> perf/core: Check return value of the perf_event_read() IPI
>
> The call to smp_call_function_single in perf_event_read() may fail if
> an invalid or not online CPU index is passed. Warn user if such bug is
> present and return error.
>
> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.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: Stephane Eranian <eranian@google.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Vegard Nossum <vegard.nossum@gmail.com>
> Cc: Vince Weaver <vincent.weaver@maine.edu>
> Link: http://lkml.kernel.org/r/1471467307-61171-2-git-send-email-davidcc@google.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> kernel/events/core.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a5fc5c8..5650f53 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3549,9 +3549,10 @@ static int perf_event_read(struct perf_event *event, bool group)
> .group = group,
> .ret = 0,
> };
> - smp_call_function_single(event->oncpu,
> - __perf_event_read, &data, 1);
> - ret = data.ret;
> + ret = smp_call_function_single(event->oncpu, __perf_event_read, &data, 1);
> + /* The event must have been read from an online CPU: */
> + WARN_ON_ONCE(ret);
> + ret = ret ? : data.ret;
> } else if (event->state == PERF_EVENT_STATE_INACTIVE) {
> struct perf_event_context *ctx = event->ctx;
> unsigned long flags;
Hi,
I'm running into this new warning now:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 23442 at kernel/events/core.c:3554
perf_event_read+0x472/0x590
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 23442 Comm: syz-executor Not tainted 4.8.0-rc2+ #145
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014
0000000000000000 ffff88010d1bf808 ffffffff81f9f201 ffffffff83e72d60
ffff88010d1bf8e0 ffffffff83ecbd00 ffffffff847070c0 ffff88010d1bf8d0
ffffffff8150538a 0000000041b58ab3 ffffffff844dc5dd ffffffff815051a7
Call Trace:
[<ffffffff81f9f201>] dump_stack+0x83/0xb2
[<ffffffff8150538a>] panic+0x1e3/0x3a1
[<ffffffff815051a7>] ? set_ti_thread_flag+0x1e/0x1e
[<ffffffff81505a58>] ? rt_mutex_top_waiter.part.10+0x6/0x6
[<ffffffff813dd140>] ? generic_exec_single+0xf0/0x2d0
[<ffffffff814d86a2>] ? perf_event_read+0x472/0x590
[<ffffffff8126df4f>] __warn+0x1bf/0x1e0
[<ffffffff8126e13c>] warn_slowpath_null+0x2c/0x40
[<ffffffff814d86a2>] perf_event_read+0x472/0x590
[<ffffffff814d8230>] ? perf_event_set_addr_filter+0xad0/0xad0
[<ffffffff814d74fd>] ? perf_event_ctx_lock_nested+0xdd/0x1e0
[<ffffffff814e8aa4>] perf_event_read_value+0x84/0x520
[<ffffffff814d75bc>] ? perf_event_ctx_lock_nested+0x19c/0x1e0
[<ffffffff814d745b>] ? perf_event_ctx_lock_nested+0x3b/0x1e0
[<ffffffff814e9383>] perf_read+0x443/0x8d0
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff81e5ae62>] ? common_file_perm+0x2e2/0x380
[<ffffffff816709a2>] do_loop_readv_writev+0x152/0x200
[<ffffffff81d935f6>] ? security_file_permission+0x86/0x1e0
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff814e8f40>] ? perf_event_read_value+0x520/0x520
[<ffffffff81673174>] do_readv_writev+0x614/0x700
[<ffffffff81672b60>] ? rw_verify_area+0x2b0/0x2b0
[<ffffffff814d7342>] ? put_ctx+0x22/0x100
[<ffffffff816d16f1>] ? __fget+0x1c1/0x270
[<ffffffff816d1577>] ? __fget+0x47/0x270
[<ffffffff81676d5b>] vfs_readv+0x8b/0xc0
[<ffffffff81676e6e>] do_readv+0xde/0x230
[<ffffffff81676d90>] ? vfs_readv+0xc0/0xc0
[<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190
[<ffffffff82001b07>] ? check_preemption_disabled+0x37/0x1e0
[<ffffffff81677617>] SyS_readv+0x27/0x30
[<ffffffff816775f0>] ? do_pwritev+0x1a0/0x1a0
[<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
[<ffffffff83c3286a>] entry_SYSCALL64_slow_path+0x25/0x25
I don't think WARN() is the right interface for signalling errors to
userspace programs?
Vegard
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2016-08-22 09:20 +0200 |
| Subject | Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI |
| Message-ID | <s8RGx-3cc-3@gated-at.bofh.it> |
| In reply to | #1466970 |
On Sun, Aug 21, 2016 at 02:10:07PM +0200, Vegard Nossum wrote: SNIP > [<ffffffff816d1577>] ? __fget+0x47/0x270 > [<ffffffff81676d5b>] vfs_readv+0x8b/0xc0 > [<ffffffff81676e6e>] do_readv+0xde/0x230 > [<ffffffff81676d90>] ? vfs_readv+0xc0/0xc0 > [<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190 > [<ffffffff82001b07>] ? check_preemption_disabled+0x37/0x1e0 > [<ffffffff81677617>] SyS_readv+0x27/0x30 > [<ffffffff816775f0>] ? do_pwritev+0x1a0/0x1a0 > [<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0 > [<ffffffff83c3286a>] entry_SYSCALL64_slow_path+0x25/0x25 > > I don't think WARN() is the right interface for signalling errors to > userspace programs? any special way to trigger that? thanks, jirka
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2016-08-22 10:30 +0200 |
| Subject | Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI |
| Message-ID | <s8SMh-3Ra-3@gated-at.bofh.it> |
| In reply to | #1467414 |
On Mon, Aug 22, 2016 at 09:17:37AM +0200, Jiri Olsa wrote: > On Sun, Aug 21, 2016 at 02:10:07PM +0200, Vegard Nossum wrote: > > SNIP > > > [<ffffffff816d1577>] ? __fget+0x47/0x270 > > [<ffffffff81676d5b>] vfs_readv+0x8b/0xc0 > > [<ffffffff81676e6e>] do_readv+0xde/0x230 > > [<ffffffff81676d90>] ? vfs_readv+0xc0/0xc0 > > [<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190 > > [<ffffffff82001b07>] ? check_preemption_disabled+0x37/0x1e0 > > [<ffffffff81677617>] SyS_readv+0x27/0x30 > > [<ffffffff816775f0>] ? do_pwritev+0x1a0/0x1a0 > > [<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0 > > [<ffffffff83c3286a>] entry_SYSCALL64_slow_path+0x25/0x25 > > > > I don't think WARN() is the right interface for signalling errors to > > userspace programs? > > any special way to trigger that? nope ;-) perf stat -a -I 10 jirka -- [ 430.168408] ------------[ cut here ]------------ [ 430.173596] WARNING: CPU: 10 PID: 3878 at kernel/events/core.c:3583 perf_event_read+0x1cc/0x1e0 [ 430.183321] Modules linked in: intel_rapl sb_edac edac_core x86_pkg_temp_thermal coretemp iTCO_wdt crct10dif_pclmul iTCO_vendor_support crc32_pclmul ghash_clmulni_intel cdc_ether usbnet mii ioatdma i2c_i801 lpc_ich i2c_smbus wmi xfs libcrc32c mgag200 drm_kms_helper igb ttm ptp pps_core crc32c_intel dca drm i2c_algo_bit megaraid_sas fjes [ 430.217193] CPU: 10 PID: 3878 Comm: perf Not tainted 4.8.0-rc2+ #227 [ 430.224282] Hardware name: IBM System x3650 M4 : -[7915E2G]-/00Y7683, BIOS -[VVE124AUS-1.30]- 11/21/2012 [ 430.234863] 0000000000000286 00000000e92a6a39 ffff880275c7bcd8 ffffffff813ba21e [ 430.243156] 0000000000000000 0000000000000000 ffff880275c7bd18 ffffffff810a16ab [ 430.251446] 00000dff75c7bd78 ffff88047444c000 ffff880275c7bde0 0000000000000003 [ 430.259736] Call Trace: [ 430.262466] [<ffffffff813ba21e>] dump_stack+0x63/0x85 [ 430.268193] [<ffffffff810a16ab>] __warn+0xcb/0xf0 [ 430.273539] [<ffffffff810a17dd>] warn_slowpath_null+0x1d/0x20 [ 430.280048] [<ffffffff8118ec7c>] perf_event_read+0x1cc/0x1e0 [ 430.286461] [<ffffffff8118ecd5>] perf_event_read_value+0x45/0x130 [ 430.293358] [<ffffffff81192a03>] perf_read+0x93/0x2c0 [ 430.299093] [<ffffffff81344879>] ? file_has_perm+0xa9/0xb0 [ 430.305314] [<ffffffff81231327>] __vfs_read+0x37/0x150 [ 430.311137] [<ffffffff8133af60>] ? security_file_permission+0xa0/0xc0 [ 430.318431] [<ffffffff81232505>] vfs_read+0x95/0x140 [ 430.324069] [<ffffffff81233a05>] SyS_read+0x55/0xc0 [ 430.329610] [<ffffffff8178b972>] entry_SYSCALL_64_fastpath+0x1a/0xa4 [ 430.336820] ---[ end trace 671c766962c86ad2 ]---
[toc] | [prev] | [next] | [standalone]
| From | Jiri Olsa <jolsa@redhat.com> |
|---|---|
| Date | 2016-08-22 12:40 +0200 |
| Subject | Re: [tip:perf/core] perf/core: Check return value of the perf_event_read() IPI |
| Message-ID | <s8UO5-56V-1@gated-at.bofh.it> |
| In reply to | #1467445 |
On Mon, Aug 22, 2016 at 10:29:32AM +0200, Jiri Olsa wrote:
> On Mon, Aug 22, 2016 at 09:17:37AM +0200, Jiri Olsa wrote:
> > On Sun, Aug 21, 2016 at 02:10:07PM +0200, Vegard Nossum wrote:
> >
> > SNIP
> >
> > > [<ffffffff816d1577>] ? __fget+0x47/0x270
> > > [<ffffffff81676d5b>] vfs_readv+0x8b/0xc0
> > > [<ffffffff81676e6e>] do_readv+0xde/0x230
> > > [<ffffffff81676d90>] ? vfs_readv+0xc0/0xc0
> > > [<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190
> > > [<ffffffff82001b07>] ? check_preemption_disabled+0x37/0x1e0
> > > [<ffffffff81677617>] SyS_readv+0x27/0x30
> > > [<ffffffff816775f0>] ? do_pwritev+0x1a0/0x1a0
> > > [<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
> > > [<ffffffff83c3286a>] entry_SYSCALL64_slow_path+0x25/0x25
> > >
> > > I don't think WARN() is the right interface for signalling errors to
> > > userspace programs?
> >
> > any special way to trigger that?
>
> nope ;-)
>
> perf stat -a -I 10
reading the event could race with event schedule out
leaving us with active state and oncpu == -1
attached patch fixes the warn for me, but I might
be missing some other cases
jirka
---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3f07e6cfc1b6..375274b6f3b4 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1802,8 +1802,9 @@ event_sched_out(struct perf_event *event,
event->tstamp_stopped = tstamp;
event->pmu->del(event, 0);
- event->oncpu = -1;
- event->state = PERF_EVENT_STATE_INACTIVE;
+ WRITE_ONCE(event->state, PERF_EVENT_STATE_INACTIVE);
+ smp_wmb();
+ WRITE_ONCE(event->oncpu, -1);
if (event->pending_disable) {
event->pending_disable = 0;
event->state = PERF_EVENT_STATE_OFF;
@@ -3424,9 +3425,8 @@ struct perf_read_data {
int ret;
};
-static int find_cpu_to_read(struct perf_event *event, int local_cpu)
+static int find_cpu_to_read(struct perf_event *event, int event_cpu, int local_cpu)
{
- int event_cpu = event->oncpu;
u16 local_pkg, event_pkg;
if (event->group_caps & PERF_EV_CAP_READ_ACTIVE_PKG) {
@@ -3561,13 +3561,17 @@ u64 perf_event_read_local(struct perf_event *event)
static int perf_event_read(struct perf_event *event, bool group)
{
- int ret = 0, cpu_to_read, local_cpu;
+ int ret = 0, cpu_to_read, local_cpu, state;
+
+ state = READ_ONCE(event->state);
+ smp_rmb();
+ cpu_to_read = event->oncpu;
/*
* If event is enabled and currently active on a CPU, update the
* value in the event structure:
*/
- if (event->state == PERF_EVENT_STATE_ACTIVE) {
+ if (state == PERF_EVENT_STATE_ACTIVE && cpu_to_read != -1) {
struct perf_read_data data = {
.event = event,
.group = group,
@@ -3575,7 +3579,7 @@ static int perf_event_read(struct perf_event *event, bool group)
};
local_cpu = get_cpu();
- cpu_to_read = find_cpu_to_read(event, local_cpu);
+ cpu_to_read = find_cpu_to_read(event, cpu_to_read, local_cpu);
put_cpu();
ret = smp_call_function_single(cpu_to_read, __perf_event_read, &data, 1);
[toc] | [prev] | [next] | [standalone]
| From | David Carrillo-Cisneros <davidcc@google.com> |
|---|---|
| Date | 2016-08-17 23:00 +0200 |
| Subject | [PATCH v3 3/4] perf/core: introduce PMU_EV_CAP_READ_ACTIVE_PKG |
| Message-ID | <s7g6m-6kR-27@gated-at.bofh.it> |
| In reply to | #1464797 |
Introduce the flag PMU_EV_CAP_READ_ACTIVE_PKG, useful for uncore events,
that allows a PMU to signal the generic perf code that an event is readable
in the current CPU if the event is active in a CPU in the same package as
the current CPU.
This is an optimization that avoids a unnecessary IPI for the common case
where uncore events are run and read in the same package but in
different CPUs.
As an example, the IPI removal speeds up perf_read in my Haswell system
as follows:
- For event UNC_C_LLC_LOOKUP: From 260 us to 31 us.
- For event RAPL's power/energy-cores/: From to 255 us to 27 us.
For the optimization to work, all events in the group must have it
(similarly to PERF_EV_CAP_SOFTWARE).
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
include/linux/perf_event.h | 3 +++
kernel/events/core.c | 25 +++++++++++++++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6f7459f..5c53625 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -514,8 +514,11 @@ typedef void (*perf_overflow_handler_t)(struct perf_event *,
* Event capabilities. For event_caps and groups caps.
*
* PERF_EV_CAP_SOFTWARE: Is a software event.
+ * PERF_EV_CAP_READ_ACTIVE_PKG: A CPU event (or cgroup event) that can be read
+ * from any CPU in the package where it is active.
*/
#define PERF_EV_CAP_SOFTWARE BIT(0)
+#define PERF_EV_CAP_READ_ACTIVE_PKG BIT(1)
#define SWEVENT_HLIST_BITS 8
#define SWEVENT_HLIST_SIZE (1 << SWEVENT_HLIST_BITS)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5b83509..06137bb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3388,6 +3388,22 @@ struct perf_read_data {
int ret;
};
+static int find_cpu_to_read(struct perf_event *event, int local_cpu)
+{
+ int event_cpu = event->oncpu;
+ u16 local_pkg, event_pkg;
+
+ if (event->group_caps & PERF_EV_CAP_READ_ACTIVE_PKG) {
+ event_pkg = topology_physical_package_id(event_cpu);
+ local_pkg = topology_physical_package_id(local_cpu);
+
+ if (event_pkg == local_pkg)
+ return local_cpu;
+ }
+
+ return event_cpu;
+}
+
/*
* Cross CPU call to read the hardware event
*/
@@ -3509,7 +3525,7 @@ u64 perf_event_read_local(struct perf_event *event)
static int perf_event_read(struct perf_event *event, bool group)
{
- int ret = 0;
+ int ret = 0, cpu_to_read, local_cpu;
/*
* If event is enabled and currently active on a CPU, update the
@@ -3521,7 +3537,12 @@ static int perf_event_read(struct perf_event *event, bool group)
.group = group,
.ret = 0,
};
- ret = smp_call_function_single(event->oncpu,
+
+ local_cpu = get_cpu();
+ cpu_to_read = find_cpu_to_read(event, local_cpu);
+ put_cpu();
+
+ ret = smp_call_function_single(cpu_to_read,
__perf_event_read, &data, 1);
/* event must have been read from an online CPU. */
WARN_ON_ONCE(ret);
--
2.8.0.rc3.226.g39d4020
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> |
|---|---|
| Date | 2016-08-18 13:50 +0200 |
| Subject | [tip:perf/core] perf/core: Introduce PMU_EV_CAP_READ_ACTIVE_PKG |
| Message-ID | <s7tZD-7Fb-9@gated-at.bofh.it> |
| In reply to | #1464800 |
Commit-ID: d6a2f9035bfc27d0e9d78b13635dda9fb017ac01
Gitweb: http://git.kernel.org/tip/d6a2f9035bfc27d0e9d78b13635dda9fb017ac01
Author: David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Wed, 17 Aug 2016 13:55:06 -0700
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 10:53:59 +0200
perf/core: Introduce PMU_EV_CAP_READ_ACTIVE_PKG
Introduce the flag PMU_EV_CAP_READ_ACTIVE_PKG, useful for uncore events,
that allows a PMU to signal the generic perf code that an event is readable
in the current CPU if the event is active in a CPU in the same package as
the current CPU.
This is an optimization that avoids a unnecessary IPI for the common case
where uncore events are run and read in the same package but in
different CPUs.
As an example, the IPI removal speeds up perf_read() in my Haswell system
as follows:
- For event UNC_C_LLC_LOOKUP: From 260 us to 31 us.
- For event RAPL's power/energy-cores/: From to 255 us to 27 us.
For the optimization to work, all events in the group must have it
(similarly to PERF_EV_CAP_SOFTWARE).
Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Carrillo-Cisneros <davidcc@google.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: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Link: http://lkml.kernel.org/r/1471467307-61171-4-git-send-email-davidcc@google.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
include/linux/perf_event.h | 3 +++
kernel/events/core.c | 25 +++++++++++++++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 6f7459f..5c53625 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -514,8 +514,11 @@ typedef void (*perf_overflow_handler_t)(struct perf_event *,
* Event capabilities. For event_caps and groups caps.
*
* PERF_EV_CAP_SOFTWARE: Is a software event.
+ * PERF_EV_CAP_READ_ACTIVE_PKG: A CPU event (or cgroup event) that can be read
+ * from any CPU in the package where it is active.
*/
#define PERF_EV_CAP_SOFTWARE BIT(0)
+#define PERF_EV_CAP_READ_ACTIVE_PKG BIT(1)
#define SWEVENT_HLIST_BITS 8
#define SWEVENT_HLIST_SIZE (1 << SWEVENT_HLIST_BITS)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8c42a5a..3f07e6c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3424,6 +3424,22 @@ struct perf_read_data {
int ret;
};
+static int find_cpu_to_read(struct perf_event *event, int local_cpu)
+{
+ int event_cpu = event->oncpu;
+ u16 local_pkg, event_pkg;
+
+ if (event->group_caps & PERF_EV_CAP_READ_ACTIVE_PKG) {
+ event_pkg = topology_physical_package_id(event_cpu);
+ local_pkg = topology_physical_package_id(local_cpu);
+
+ if (event_pkg == local_pkg)
+ return local_cpu;
+ }
+
+ return event_cpu;
+}
+
/*
* Cross CPU call to read the hardware event
*/
@@ -3545,7 +3561,7 @@ u64 perf_event_read_local(struct perf_event *event)
static int perf_event_read(struct perf_event *event, bool group)
{
- int ret = 0;
+ int ret = 0, cpu_to_read, local_cpu;
/*
* If event is enabled and currently active on a CPU, update the
@@ -3557,7 +3573,12 @@ static int perf_event_read(struct perf_event *event, bool group)
.group = group,
.ret = 0,
};
- ret = smp_call_function_single(event->oncpu, __perf_event_read, &data, 1);
+
+ local_cpu = get_cpu();
+ cpu_to_read = find_cpu_to_read(event, local_cpu);
+ put_cpu();
+
+ ret = smp_call_function_single(cpu_to_read, __perf_event_read, &data, 1);
/* The event must have been read from an online CPU: */
WARN_ON_ONCE(ret);
ret = ret ? : data.ret;
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web