[PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Namhyung Kim]

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@vger.kernel.org
Cc: x86@kernel.org
Cc: bpf@vger.kernel.org
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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, &regs);
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, &regs);
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, &regs);
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

Re: [PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Peter Zijlstra]

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.

Re: [PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Song Liu]

On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <namhyung@kernel.org> 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@vger.kernel.org
> Cc: x86@kernel.org
> Cc: bpf@vger.kernel.org
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  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, &regs);
> 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, &regs);
> 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, &regs);
> 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
>

Re: [PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Namhyung Kim]

Hi Song,

On Fri, Jan 13, 2023 at 1:01 PM Song Liu <song@kernel.org> wrote:
>
> On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <namhyung@kernel.org> 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@vger.kernel.org
> > Cc: x86@kernel.org
> > Cc: bpf@vger.kernel.org
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---

[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

Re: [PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Song Liu]

On Fri, Jan 13, 2023 at 1:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Song,
>
> On Fri, Jan 13, 2023 at 1:01 PM Song Liu <song@kernel.org> wrote:
> >
> > On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <namhyung@kernel.org> 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@vger.kernel.org
> > > Cc: x86@kernel.org
> > > Cc: bpf@vger.kernel.org
> > > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
>
> [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

[PATCH 7/8] perf/core: Introduce perf_prepare_header()

[Namhyung Kim]

Factor out perf_prepare_header() so that it can call
perf_prepare_sample() without a header if not needed.

Also it checks the filtered_sample_type to avoid duplicate
work when perf_prepare_sample() is called twice (or more).

Cc: linux-s390@vger.kernel.org
Cc: x86@kernel.org
Suggested-by: Peter Zijlstr <peterz@infradead.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 arch/s390/kernel/perf_cpum_sf.c |  3 ++-
 arch/x86/events/intel/ds.c      |  3 ++-
 include/linux/perf_event.h      | 16 +++++++++++++-
 kernel/events/core.c            | 38 +++++++++++++++++++++------------
 4 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
index 332a49965130..fd02f8423243 100644
--- a/arch/s390/kernel/perf_cpum_sf.c
+++ b/arch/s390/kernel/perf_cpum_sf.c
@@ -671,7 +671,8 @@ static void cpumsf_output_event_pid(struct perf_event *event,
 	/* Protect callchain buffers, tasks */
 	rcu_read_lock();
 
-	perf_prepare_sample(&header, data, event, regs);
+	perf_prepare_sample(data, event, regs);
+	perf_prepare_header(&header, data, event, regs);
 	if (perf_output_begin(&handle, data, event, header.size))
 		goto out;
 
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 07c8a2cdc3ee..183efa914b99 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -807,7 +807,8 @@ int intel_pmu_drain_bts_buffer(void)
 	 * the sample.
 	 */
 	rcu_read_lock();
-	perf_prepare_sample(&header, &data, event, &regs);
+	perf_prepare_sample(&data, event, &regs);
+	perf_prepare_header(&header, &data, event, &regs);
 
 	if (perf_output_begin(&handle, &data, event,
 			      header.size * (top - base - skip)))
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7db0e9cc2682..d5628a7b5eaa 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1250,6 +1250,17 @@ static inline void perf_sample_save_brstack(struct perf_sample_data *data,
 	data->sample_flags |= PERF_SAMPLE_BRANCH_STACK;
 }
 
+static inline u32 perf_sample_data_size(struct perf_sample_data *data,
+					struct perf_event *event)
+{
+	u32 size = sizeof(struct perf_event_header);
+
+	size += event->header_size + event->id_header_size;
+	size += data->dyn_size;
+
+	return size;
+}
+
 /*
  * Clear all bitfields in the perf_branch_entry.
  * The to and from fields are not cleared because they are
@@ -1271,7 +1282,10 @@ extern void perf_output_sample(struct perf_output_handle *handle,
 			       struct perf_event_header *header,
 			       struct perf_sample_data *data,
 			       struct perf_event *event);
-extern void perf_prepare_sample(struct perf_event_header *header,
+extern void perf_prepare_sample(struct perf_sample_data *data,
+				struct perf_event *event,
+				struct pt_regs *regs);
+extern void perf_prepare_header(struct perf_event_header *header,
 				struct perf_sample_data *data,
 				struct perf_event *event,
 				struct pt_regs *regs);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 127dddd20f93..5c4f3fa3d2b7 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7574,20 +7574,13 @@ static __always_inline u64 __cond_set(u64 flags, u64 s, u64 d)
 	return d * !!(flags & s);
 }
 
-void perf_prepare_sample(struct perf_event_header *header,
-			 struct perf_sample_data *data,
+void perf_prepare_sample(struct perf_sample_data *data,
 			 struct perf_event *event,
 			 struct pt_regs *regs)
 {
 	u64 sample_type = event->attr.sample_type;
 	u64 filtered_sample_type;
 
-	header->type = PERF_RECORD_SAMPLE;
-	header->size = sizeof(*header) + event->header_size + event->id_header_size;
-
-	header->misc = 0;
-	header->misc |= perf_misc_flags(regs);
-
 	/*
 	 * Add the sample flags that are dependent to others.  And clear the
 	 * sample flags that have already been done by the PMU driver.
@@ -7601,6 +7594,12 @@ void perf_prepare_sample(struct perf_event_header *header,
 					   PERF_SAMPLE_REGS_USER);
 	filtered_sample_type &= ~data->sample_flags;
 
+	if (filtered_sample_type == 0) {
+		/* Make sure it has the correct data->type for output */
+		data->type = event->attr.sample_type;
+		return;
+	}
+
 	__perf_event_header__init_id(data, event, filtered_sample_type);
 	data->sample_flags |= sample_type & PERF_SAMPLE_ID_ALL;
 
@@ -7653,9 +7652,10 @@ void perf_prepare_sample(struct perf_event_header *header,
 		 * up the rest of the sample size.
 		 */
 		u16 stack_size = event->attr.sample_stack_user;
+		u16 header_size = perf_sample_data_size(data, event);
 		u16 size = sizeof(u64);
 
-		stack_size = perf_sample_ustack_size(stack_size, header->size,
+		stack_size = perf_sample_ustack_size(stack_size, header_size,
 						     data->regs_user.regs);
 
 		/*
@@ -7740,8 +7740,9 @@ void perf_prepare_sample(struct perf_event_header *header,
 
 	if (filtered_sample_type & PERF_SAMPLE_AUX) {
 		u64 size;
+		u16 header_size = perf_sample_data_size(data, event);
 
-		header->size += sizeof(u64); /* size */
+		header_size += sizeof(u64); /* size */
 
 		/*
 		 * Given the 16bit nature of header::size, an AUX sample can
@@ -7749,17 +7750,25 @@ 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,
+		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);
 
-		WARN_ON_ONCE(size + header->size > U16_MAX);
+		WARN_ON_ONCE(size + header_size > U16_MAX);
 		data->dyn_size += size + sizeof(u64); /* size above */
 		data->sample_flags |= PERF_SAMPLE_AUX;
 	}
+}
 
-	header->size += data->dyn_size;
+void perf_prepare_header(struct perf_event_header *header,
+			 struct perf_sample_data *data,
+			 struct perf_event *event,
+			 struct pt_regs *regs)
+{
+	header->type = PERF_RECORD_SAMPLE;
+	header->size = perf_sample_data_size(data, event);
+	header->misc = perf_misc_flags(regs);
 
 	/*
 	 * If you're adding more sample types here, you likely need to do
@@ -7788,7 +7797,8 @@ __perf_event_output(struct perf_event *event,
 	/* protect the callchain buffers */
 	rcu_read_lock();
 
-	perf_prepare_sample(&header, data, event, regs);
+	perf_prepare_sample(data, event, regs);
+	perf_prepare_header(&header, data, event, regs);
 
 	err = output_begin(&handle, data, event, header.size);
 	if (err)
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH 7/8] perf/core: Introduce perf_prepare_header()

[Song Liu]

On Thu, Jan 12, 2023 at 1:40 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Factor out perf_prepare_header() so that it can call
> perf_prepare_sample() without a header if not needed.
>
> Also it checks the filtered_sample_type to avoid duplicate
> work when perf_prepare_sample() is called twice (or more).
>
> Cc: linux-s390@vger.kernel.org
> Cc: x86@kernel.org
> Suggested-by: Peter Zijlstr <peterz@infradead.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Song Liu <song@kernel.org>

[PATCH 3/8] perf/core: Add perf_sample_save_raw_data() helper

[Namhyung Kim]

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@vger.kernel.org
Cc: x86@kernel.org
Cc: bpf@vger.kernel.org
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Jiri Olsa <jolsa@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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, &regs);
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, &regs);
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, &regs);
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