Hi Ian,
On Thu, May 29, 2025 at 10:47 AM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> > + bpf_metadata_event = &metadata->event->bpf_metadata;
> > + *bpf_metadata_event = (struct perf_record_bpf_metadata) {
> > + .header = {
> > + .type = PERF_RECORD_BPF_METADATA,
> > + .size = metadata->event_size,
>
> nit: Could we set the header.size in bpf_metadata_alloc to remove
> metadata->event_size. The code generally doesn't pass a pair of
> perf_event + size around as the size should be in the header.
I can do this initialization of metadata->event->bpf_metadata in
bpf_metadata_alloc(). I'll need to do a bit more work in
synthesize_perf_record_bpf_metadata() before I can get rid of
metadata->event_size, because it needs to allocate an additional
machine->id_hdr_size bytes (see below); I'd just have it get the
value out of metadata->event->header.size instead. Sound good?
Blake
> > +static int synthesize_perf_record_bpf_metadata(const struct bpf_metadata *metadata,
> > + const struct perf_tool *tool,
> > + perf_event__handler_t process,
> > + struct machine *machine)
> > +{
> > + union perf_event *event;
> > + int err = 0;
> > +
> > + event = calloc(1, metadata->event_size + machine->id_hdr_size);
> > + if (!event)
> > + return -1;
> > + memcpy(event, metadata->event, metadata->event_size);
> > + memset((void *)event + event->header.size, 0, machine->id_hdr_size);
> > + event->header.size += machine->id_hdr_size;
> > + for (__u32 index = 0; index < metadata->nr_prog_names; index++) {
> > + memcpy(event->bpf_metadata.prog_name,
> > + metadata->prog_names[index], BPF_PROG_NAME_LEN);
> > + err = perf_tool__process_synth_event(tool, event, machine,
> > + process);
> > + if (err != 0)
> > + break;
> > + }
> > +
> > + free(event);
> > + return err;
> > +}
