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: [email protected]
Cc: [email protected]
Cc: [email protected]
Suggested-by: Peter Zijlstra <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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;
}
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
On Thu, Jan 12, 2023 at 01:40:10PM -0800, Namhyung Kim wrote:
> @@ -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;
> +}
This might be a wee big for inline, but I suppose it doesn't matter too
much.
On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <[email protected]> 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: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Suggested-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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
>
Hi Song,
On Fri, Jan 13, 2023 at 1:01 PM Song Liu <[email protected]> wrote:
>
> On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <[email protected]> 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: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Suggested-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Namhyung Kim <[email protected]>
> > ---
[SNIP]
> > 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.
It's for when the user requested RAW but no actual data.
It assumes PMU drivers call perf_sample_save_raw_data()
before perf_prepare_sample() if there's RAW data.
So we can only handle the 'else' part in the original code.
Thanks,
Namhyung
On Fri, Jan 13, 2023 at 1:56 PM Namhyung Kim <[email protected]> wrote:
>
> Hi Song,
>
> On Fri, Jan 13, 2023 at 1:01 PM Song Liu <[email protected]> wrote:
> >
> > On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <[email protected]> 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: [email protected]
> > > Cc: [email protected]
> > > Cc: [email protected]
> > > Suggested-by: Peter Zijlstra <[email protected]>
> > > Signed-off-by: Namhyung Kim <[email protected]>
> > > ---
>
> [SNIP]
> > > 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.
>
> It's for when the user requested RAW but no actual data.
> It assumes PMU drivers call perf_sample_save_raw_data()
> before perf_prepare_sample() if there's RAW data.
> So we can only handle the 'else' part in the original code.
Got it. Thanks for the explanation.
Song