> On 28-Sep-2022, at 12:27 PM, tip-bot2 for Namhyung Kim <tip-bot2@xxxxxxxxxxxxx> wrote:
>
> The following commit has been merged into the perf/core branch of tip:
>
> Commit-ID: 838d9bb62d132ec3baf1b5aba2e95ef9a7a9a3cd
> Gitweb: https://git.kernel.org/tip/838d9bb62d132ec3baf1b5aba2e95ef9a7a9a3cd
> Author: Namhyung Kim <namhyung@xxxxxxxxxx>
> AuthorDate: Wed, 21 Sep 2022 15:00:32 -07:00
> Committer: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> CommitterDate: Tue, 27 Sep 2022 22:50:24 +02:00
>
> perf: Use sample_flags for raw_data
>
> Use the new sample_flags to indicate whether the raw data field is
> filled by the PMU driver. Although it could check with the NULL,
> follow the same rule with other fields.
>
> Remove the raw field from the perf_sample_data_init() to minimize
> the number of cache lines touched.
>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Link: https://lkml.kernel.org/r/20220921220032.2858517-2-namhyung@xxxxxxxxxx
Hi Namhyung,
This commit ("perf: Use sample_flags for raw_data") added
PERF_SAMPLE_RAW check in perf_prepare_sample. To be in sync
while we output sample to memory, do we also need to add
similar check in perf_output_sample ? I am pasting change below.
Please share your thoughts.
>From 46d874bc4a915dd710ddbc5198588cbb66d3ea8e Mon Sep 17 00:00:00 2001
From: Athira Rajeev <atrajeev@xxxxxxxxxxxxxxxxxx>
Date: Wed, 19 Oct 2022 13:02:06 +0530
Subject: [PATCH] perf/core: Update sample_flags for raw_data in
perf_output_sample
commit 838d9bb62d13 ("perf: Use sample_flags for raw_data")
added check for PERF_SAMPLE_RAW in sample_flags in
perf_prepare_sample(). But while copying the sample in memory,
the check for sample_flags is not added in perf_output_sample().
Fix adds the same in perf_output_sample as well.
Fixes: 838d9bb62d13 ("perf: Use sample_flags for raw_data")
Signed-off-by: Athira Rajeev <atrajeev@xxxxxxxxxxxxxxxxxx>
---
kernel/events/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4ec3717003d5..daf387c75d33 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7099,7 +7099,7 @@ void perf_output_sample(struct perf_output_handle *handle,
if (sample_type & PERF_SAMPLE_RAW) {
struct perf_raw_record *raw = data->raw;
- if (raw) {
+ if (raw && (data->sample_flags & PERF_SAMPLE_RAW)) {
struct perf_raw_frag *frag = &raw->frag;
perf_output_put(handle, raw->size);
--
2.31.1
Thanks
Athira
> ---
> arch/s390/kernel/perf_cpum_cf.c | 1 +
> arch/s390/kernel/perf_pai_crypto.c | 1 +
> arch/x86/events/amd/ibs.c | 1 +
> include/linux/perf_event.h | 5 ++---
> kernel/events/core.c | 3 ++-
> 5 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
> index f7dd3c8..f043a7f 100644
> --- a/arch/s390/kernel/perf_cpum_cf.c
> +++ b/arch/s390/kernel/perf_cpum_cf.c
> @@ -664,6 +664,7 @@ static int cfdiag_push_sample(struct perf_event *event,
> raw.frag.data = cpuhw->stop;
> raw.size = raw.frag.size;
> data.raw = &raw;
> + data.sample_flags |= PERF_SAMPLE_RAW;
> }
>
> overflow = perf_event_overflow(event, &data, ®s);
> diff --git a/arch/s390/kernel/perf_pai_crypto.c b/arch/s390/kernel/perf_pai_crypto.c
> index b38b4ae..6826e2a 100644
> --- a/arch/s390/kernel/perf_pai_crypto.c
> +++ b/arch/s390/kernel/perf_pai_crypto.c
> @@ -366,6 +366,7 @@ static int paicrypt_push_sample(void)
> raw.frag.data = cpump->save;
> raw.size = raw.frag.size;
> data.raw = &raw;
> + data.sample_flags |= PERF_SAMPLE_RAW;
> }
>
> overflow = perf_event_overflow(event, &data, ®s);
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index ce5720b..c29a006 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -781,6 +781,7 @@ fail:
> },
> };
> data.raw = &raw;
> + data.sample_flags |= PERF_SAMPLE_RAW;
> }
>
> /*
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index f4a1357..e9b151c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1028,7 +1028,6 @@ struct perf_sample_data {
> * minimize the cachelines touched.
> */
> u64 sample_flags;
> - struct perf_raw_record *raw;
> u64 period;
>
> /*
> @@ -1040,6 +1039,7 @@ struct perf_sample_data {
> union perf_mem_data_src data_src;
> u64 txn;
> u64 addr;
> + struct perf_raw_record *raw;
>
> u64 type;
> u64 ip;
> @@ -1078,8 +1078,7 @@ static inline void perf_sample_data_init(struct perf_sample_data *data,
> u64 addr, u64 period)
> {
> /* remaining struct members initialized in perf_prepare_sample() */
> - data->sample_flags = 0;
> - data->raw = NULL;
> + data->sample_flags = PERF_SAMPLE_PERIOD;
> data->period = period;
>
> if (addr) {
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a91f74d..04e19a8 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7332,7 +7332,7 @@ void perf_prepare_sample(struct perf_event_header *header,
> struct perf_raw_record *raw = data->raw;
> int size;
>
> - if (raw) {
> + if (raw && (data->sample_flags & PERF_SAMPLE_RAW)) {
> struct perf_raw_frag *frag = &raw->frag;
> u32 sum = 0;
>
> @@ -7348,6 +7348,7 @@ void perf_prepare_sample(struct perf_event_header *header,
> frag->pad = raw->size - sum;
> } else {
> size = sizeof(u64);
> + data->raw = NULL;
> }
>
> header->size += size;
|