From: Namhyung Kim <namhyung@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@kernel.org>,
Kan Liang <kan.liang@linux.intel.com>,
Ravi Bangoria <ravi.bangoria@amd.com>,
bpf@vger.kernel.org
Subject: [PATCH 2/3] perf/core: Set data->sample_flags in perf_prepare_sample()
Date: Thu, 29 Dec 2022 12:41:00 -0800 [thread overview]
Message-ID: <20221229204101.1099430-2-namhyung@kernel.org> (raw)
In-Reply-To: <20221229204101.1099430-1-namhyung@kernel.org>
The perf_prepare_sample() sets the perf_sample_data according to the
attr->sample_type before copying it to the ring buffer. But BPF also
wants to access the sample data so it needs to prepare the sample even
before the regular path.
That means the perf_prepare_sample() can be called more than once. Set
the data->sample_flags consistently so that it can indicate which fields
are set already and skip them if sets.
Mostly it's just a matter of checking filtered_sample_type which is a
bitmask for unset bits in the attr->sample_type. But some of sample
data is implied by others even if it's not in the attr->sample_type
(like PERF_SAMPLE_ADDR for PERF_SAMPLE_PHYS_ADDR). So they need to
check data->sample_flags separately.
Also some of them like callchain, user regs/stack and aux data require
more calculations. Protect them using the data->sample_flags to avoid
the duplicate work.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
Maybe we don't need this change to prevent duplication in favor of the
next patch using the data->saved_size. But I think it's still useful
to set data->sample_flags consistently. Anyway it's up to you.
kernel/events/core.c | 86 ++++++++++++++++++++++++++++++++------------
1 file changed, 63 insertions(+), 23 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index eacc3702654d..70bff8a04583 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7582,14 +7582,21 @@ void perf_prepare_sample(struct perf_event_header *header,
filtered_sample_type = sample_type & ~data->sample_flags;
__perf_event_header__init_id(header, data, event, filtered_sample_type);
- if (sample_type & (PERF_SAMPLE_IP | PERF_SAMPLE_CODE_PAGE_SIZE))
- data->ip = perf_instruction_pointer(regs);
+ if (sample_type & (PERF_SAMPLE_IP | PERF_SAMPLE_CODE_PAGE_SIZE)) {
+ /* attr.sample_type may not have PERF_SAMPLE_IP */
+ if (!(data->sample_flags & PERF_SAMPLE_IP)) {
+ data->ip = perf_instruction_pointer(regs);
+ data->sample_flags |= PERF_SAMPLE_IP;
+ }
+ }
if (sample_type & PERF_SAMPLE_CALLCHAIN) {
int size = 1;
- if (filtered_sample_type & PERF_SAMPLE_CALLCHAIN)
+ if (filtered_sample_type & PERF_SAMPLE_CALLCHAIN) {
data->callchain = perf_callchain(event, regs);
+ data->sample_flags |= PERF_SAMPLE_CALLCHAIN;
+ }
size += data->callchain->nr;
@@ -7634,8 +7641,13 @@ void perf_prepare_sample(struct perf_event_header *header,
header->size += size;
}
- if (sample_type & (PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER))
- perf_sample_regs_user(&data->regs_user, regs);
+ if (sample_type & (PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER)) {
+ /* attr.sample_type may not have PERF_SAMPLE_REGS_USER */
+ if (!(data->sample_flags & PERF_SAMPLE_REGS_USER)) {
+ perf_sample_regs_user(&data->regs_user, regs);
+ data->sample_flags |= PERF_SAMPLE_REGS_USER;
+ }
+ }
if (sample_type & PERF_SAMPLE_REGS_USER) {
/* regs dump ABI info */
@@ -7656,11 +7668,18 @@ void perf_prepare_sample(struct perf_event_header *header,
* in case new sample type is added, because we could eat
* up the rest of the sample size.
*/
- u16 stack_size = event->attr.sample_stack_user;
u16 size = sizeof(u64);
+ u16 stack_size;
+
+ if (filtered_sample_type & PERF_SAMPLE_STACK_USER) {
+ stack_size = event->attr.sample_stack_user;
+ stack_size = perf_sample_ustack_size(stack_size, header->size,
+ data->regs_user.regs);
- stack_size = perf_sample_ustack_size(stack_size, header->size,
- data->regs_user.regs);
+ data->stack_user_size = stack_size;
+ data->sample_flags |= PERF_SAMPLE_STACK_USER;
+ }
+ stack_size = data->stack_user_size;
/*
* If there is something to dump, add space for the dump
@@ -7670,29 +7689,40 @@ void perf_prepare_sample(struct perf_event_header *header,
if (stack_size)
size += sizeof(u64) + stack_size;
- data->stack_user_size = stack_size;
header->size += size;
}
- if (filtered_sample_type & PERF_SAMPLE_WEIGHT_TYPE)
+ if (filtered_sample_type & PERF_SAMPLE_WEIGHT_TYPE) {
data->weight.full = 0;
+ data->sample_flags |= PERF_SAMPLE_WEIGHT_TYPE;
+ }
- if (filtered_sample_type & PERF_SAMPLE_DATA_SRC)
+ if (filtered_sample_type & PERF_SAMPLE_DATA_SRC) {
data->data_src.val = PERF_MEM_NA;
+ data->sample_flags |= PERF_SAMPLE_DATA_SRC;
+ }
- if (filtered_sample_type & PERF_SAMPLE_TRANSACTION)
+ if (filtered_sample_type & PERF_SAMPLE_TRANSACTION) {
data->txn = 0;
+ data->sample_flags |= PERF_SAMPLE_TRANSACTION;
+ }
if (sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR | PERF_SAMPLE_DATA_PAGE_SIZE)) {
- if (filtered_sample_type & PERF_SAMPLE_ADDR)
+ /* attr.sample_type may not have PERF_SAMPLE_ADDR */
+ if (!(data->sample_flags & PERF_SAMPLE_ADDR)) {
data->addr = 0;
+ data->sample_flags |= PERF_SAMPLE_ADDR;
+ }
}
if (sample_type & PERF_SAMPLE_REGS_INTR) {
/* regs dump ABI info */
int size = sizeof(u64);
- perf_sample_regs_intr(&data->regs_intr, regs);
+ if (filtered_sample_type & PERF_SAMPLE_REGS_INTR) {
+ perf_sample_regs_intr(&data->regs_intr, regs);
+ data->sample_flags |= PERF_SAMPLE_REGS_INTR;
+ }
if (data->regs_intr.regs) {
u64 mask = event->attr.sample_regs_intr;
@@ -7703,17 +7733,19 @@ void perf_prepare_sample(struct perf_event_header *header,
header->size += size;
}
- if (sample_type & PERF_SAMPLE_PHYS_ADDR &&
- filtered_sample_type & PERF_SAMPLE_PHYS_ADDR)
+ if (filtered_sample_type & PERF_SAMPLE_PHYS_ADDR) {
data->phys_addr = perf_virt_to_phys(data->addr);
+ data->sample_flags |= PERF_SAMPLE_PHYS_ADDR;
+ }
#ifdef CONFIG_CGROUP_PERF
- if (sample_type & PERF_SAMPLE_CGROUP) {
+ if (filtered_sample_type & PERF_SAMPLE_CGROUP) {
struct cgroup *cgrp;
/* protected by RCU */
cgrp = task_css_check(current, perf_event_cgrp_id, 1)->cgroup;
data->cgroup = cgroup_id(cgrp);
+ data->sample_flags |= PERF_SAMPLE_CGROUP;
}
#endif
@@ -7722,11 +7754,15 @@ void perf_prepare_sample(struct perf_event_header *header,
* require PERF_SAMPLE_ADDR, kernel implicitly retrieve the data->addr,
* but the value will not dump to the userspace.
*/
- if (sample_type & PERF_SAMPLE_DATA_PAGE_SIZE)
+ if (filtered_sample_type & PERF_SAMPLE_DATA_PAGE_SIZE) {
data->data_page_size = perf_get_page_size(data->addr);
+ data->sample_flags |= PERF_SAMPLE_DATA_PAGE_SIZE;
+ }
- if (sample_type & PERF_SAMPLE_CODE_PAGE_SIZE)
+ if (filtered_sample_type & PERF_SAMPLE_CODE_PAGE_SIZE) {
data->code_page_size = perf_get_page_size(data->ip);
+ data->sample_flags |= PERF_SAMPLE_CODE_PAGE_SIZE;
+ }
if (sample_type & PERF_SAMPLE_AUX) {
u64 size;
@@ -7739,10 +7775,14 @@ void perf_prepare_sample(struct perf_event_header *header,
* Make sure this doesn't happen by using up to U16_MAX bytes
* per sample in total (rounded down to 8 byte boundary).
*/
- size = min_t(size_t, U16_MAX - header->size,
- event->attr.aux_sample_size);
- size = rounddown(size, 8);
- size = perf_prepare_sample_aux(event, data, size);
+ if (filtered_sample_type & PERF_SAMPLE_AUX) {
+ size = min_t(size_t, U16_MAX - header->size,
+ event->attr.aux_sample_size);
+ size = rounddown(size, 8);
+ perf_prepare_sample_aux(event, data, size);
+ data->sample_flags |= PERF_SAMPLE_AUX;
+ }
+ size = data->aux_size;
WARN_ON_ONCE(size + header->size > U16_MAX);
header->size += size;
--
2.39.0.314.g84b9a713c41-goog
next prev parent reply other threads:[~2022-12-29 20:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-29 20:40 [PATCH 1/3] perf/core: Change the layout of perf_sample_data Namhyung Kim
2022-12-29 20:41 ` Namhyung Kim [this message]
2023-01-09 12:14 ` [PATCH 2/3] perf/core: Set data->sample_flags in perf_prepare_sample() Peter Zijlstra
2023-01-09 20:21 ` Namhyung Kim
2023-01-10 10:54 ` Peter Zijlstra
2023-01-10 11:10 ` Ingo Molnar
2023-01-10 19:00 ` Namhyung Kim
2023-01-10 10:55 ` Peter Zijlstra
2023-01-10 19:01 ` Namhyung Kim
2023-01-10 20:06 ` Namhyung Kim
2023-01-11 12:54 ` Peter Zijlstra
2023-01-11 16:45 ` Peter Zijlstra
2023-01-11 17:59 ` Namhyung Kim
2022-12-29 20:41 ` [PATCH 3/3] perf/core: Save calculated sample data size Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221229204101.1099430-2-namhyung@kernel.org \
--to=namhyung@kernel.org \
--cc=acme@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox