On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> When it saves the raw_data to the perf sample data, it needs to update
> the sample flags and the dynamic size. To make sure this, add the
> perf_sample_save_raw_data() helper and convert all call sites.
>
> Cc: linux-s390@xxxxxxxxxxxxxxx
> Cc: x86@xxxxxxxxxx
> Cc: bpf@xxxxxxxxxxxxxxx
> Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> arch/s390/kernel/perf_cpum_cf.c | 4 +---
> arch/s390/kernel/perf_pai_crypto.c | 4 +---
> arch/s390/kernel/perf_pai_ext.c | 4 +---
> arch/x86/events/amd/ibs.c | 3 +--
> include/linux/perf_event.h | 33 +++++++++++++++++++++++++-----
> kernel/events/core.c | 31 +++++-----------------------
> kernel/trace/bpf_trace.c | 6 ++----
> 7 files changed, 39 insertions(+), 46 deletions(-)
>
> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
> index f043a7ff220b..aa38649c7c27 100644
> --- a/arch/s390/kernel/perf_cpum_cf.c
> +++ b/arch/s390/kernel/perf_cpum_cf.c
> @@ -662,9 +662,7 @@ static int cfdiag_push_sample(struct perf_event *event,
> if (event->attr.sample_type & PERF_SAMPLE_RAW) {
> raw.frag.size = cpuhw->usedss;
> raw.frag.data = cpuhw->stop;
> - raw.size = raw.frag.size;
> - data.raw = &raw;
> - data.sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(&data, &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 985e243a2ed8..a7b339c4fd7c 100644
> --- a/arch/s390/kernel/perf_pai_crypto.c
> +++ b/arch/s390/kernel/perf_pai_crypto.c
> @@ -362,9 +362,7 @@ static int paicrypt_push_sample(void)
> if (event->attr.sample_type & PERF_SAMPLE_RAW) {
> raw.frag.size = rawsize;
> raw.frag.data = cpump->save;
> - raw.size = raw.frag.size;
> - data.raw = &raw;
> - data.sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(&data, &raw);
> }
>
> overflow = perf_event_overflow(event, &data, ®s);
> diff --git a/arch/s390/kernel/perf_pai_ext.c b/arch/s390/kernel/perf_pai_ext.c
> index 1138f57baae3..555597222bad 100644
> --- a/arch/s390/kernel/perf_pai_ext.c
> +++ b/arch/s390/kernel/perf_pai_ext.c
> @@ -451,9 +451,7 @@ static int paiext_push_sample(void)
> if (event->attr.sample_type & PERF_SAMPLE_RAW) {
> raw.frag.size = rawsize;
> raw.frag.data = cpump->save;
> - raw.size = raw.frag.size;
> - data.raw = &raw;
> - data.sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(&data, &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 417c80bd3274..64582954b5f6 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -1110,8 +1110,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
> .data = ibs_data.data,
> },
> };
> - data.raw = &raw;
> - data.sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(&data, &raw);
> }
>
> if (perf_ibs == &perf_ibs_op)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index a9419608402b..569dfac5887f 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -95,6 +95,11 @@ struct perf_raw_record {
> u32 size;
> };
>
> +static __always_inline bool perf_raw_frag_last(const struct perf_raw_frag *frag)
> +{
> + return frag->pad < sizeof(u64);
> +}
> +
> /*
> * branch stack layout:
> * nr: number of taken branches stored in entries[]
> @@ -1182,6 +1187,29 @@ static inline void perf_sample_save_callchain(struct perf_sample_data *data,
> data->sample_flags |= PERF_SAMPLE_CALLCHAIN;
> }
>
> +static inline void perf_sample_save_raw_data(struct perf_sample_data *data,
> + struct perf_raw_record *raw)
> +{
> + struct perf_raw_frag *frag = &raw->frag;
> + u32 sum = 0;
> + int size;
> +
> + do {
> + sum += frag->size;
> + if (perf_raw_frag_last(frag))
> + break;
> + frag = frag->next;
> + } while (1);
> +
> + size = round_up(sum + sizeof(u32), sizeof(u64));
> + raw->size = size - sizeof(u32);
> + frag->pad = raw->size - sum;
> +
> + data->raw = raw;
> + data->dyn_size += size;
> + data->sample_flags |= PERF_SAMPLE_RAW;
> +}
> +
> /*
> * Clear all bitfields in the perf_branch_entry.
> * The to and from fields are not cleared because they are
> @@ -1690,11 +1718,6 @@ extern void perf_restore_debug_store(void);
> static inline void perf_restore_debug_store(void) { }
> #endif
>
> -static __always_inline bool perf_raw_frag_last(const struct perf_raw_frag *frag)
> -{
> - return frag->pad < sizeof(u64);
> -}
> -
> #define perf_output_put(handle, x) perf_output_copy((handle), &(x), sizeof(x))
>
> struct perf_pmu_events_attr {
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 0fba98b9cd65..133894ae5e30 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7588,30 +7588,10 @@ void perf_prepare_sample(struct perf_event_header *header,
> if (filtered_sample_type & PERF_SAMPLE_CALLCHAIN)
> perf_sample_save_callchain(data, event, regs);
>
> - if (sample_type & PERF_SAMPLE_RAW) {
> - struct perf_raw_record *raw = data->raw;
> - int size;
> -
> - if (raw && (data->sample_flags & PERF_SAMPLE_RAW)) {
> - struct perf_raw_frag *frag = &raw->frag;
> - u32 sum = 0;
> -
> - do {
> - sum += frag->size;
> - if (perf_raw_frag_last(frag))
> - break;
> - frag = frag->next;
> - } while (1);
> -
> - size = round_up(sum + sizeof(u32), sizeof(u64));
> - raw->size = size - sizeof(u32);
> - frag->pad = raw->size - sum;
> - } else {
> - size = sizeof(u64);
> - data->raw = NULL;
> - }
> -
> - data->dyn_size += size;
> + if (filtered_sample_type & PERF_SAMPLE_RAW) {
> + data->raw = NULL;
> + data->dyn_size += sizeof(u64);
> + data->sample_flags |= PERF_SAMPLE_RAW;
> }
I don't quite follow this change, and the commit log doesn't seem
to cover this part.
>
> if (sample_type & PERF_SAMPLE_BRANCH_STACK) {
> @@ -10127,8 +10107,7 @@ void perf_tp_event(u16 event_type, u64 count, void *record, int entry_size,
> };
>
> perf_sample_data_init(&data, 0, 0);
> - data.raw = &raw;
> - data.sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(&data, &raw);
>
> perf_trace_buf_update(record, event_type);
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 3bbd3f0c810c..ad37608afc35 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -687,8 +687,7 @@ BPF_CALL_5(bpf_perf_event_output, struct pt_regs *, regs, struct bpf_map *, map,
> }
>
> perf_sample_data_init(sd, 0, 0);
> - sd->raw = &raw;
> - sd->sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(sd, &raw);
>
> err = __bpf_perf_event_output(regs, map, flags, sd);
>
> @@ -746,8 +745,7 @@ u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
>
> perf_fetch_caller_regs(regs);
> perf_sample_data_init(sd, 0, 0);
> - sd->raw = &raw;
> - sd->sample_flags |= PERF_SAMPLE_RAW;
> + perf_sample_save_raw_data(sd, &raw);
>
> ret = __bpf_perf_event_output(regs, map, flags, sd);
> out:
> --
> 2.39.0.314.g84b9a713c41-goog
>
