linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Song Liu <songliubraving@fb.com>
Cc: linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	kernel-team@fb.com, Jiri Olsa <jolsa@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [PATCH perf] perf tools: fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT
Date: Thu, 24 Jan 2019 11:29:15 +0100	[thread overview]
Message-ID: <20190124102915.GA30768@kernel.org> (raw)
In-Reply-To: <20190124094946.GK14973@kernel.org>

Em Thu, Jan 24, 2019 at 10:49:46AM +0100, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 22, 2019 at 01:02:18PM -0800, Song Liu escreveu:
> > Added missing machine->id_hdr_size to event->header.size. Also fixed size
> > of PERF_RECORD_KSYMBOL by removing extra bytes for name.
> 
> Ok, noticed this now, thanks,
> 
> - Arnaldo
>  
> > Fixes: 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Suggested-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Song Liu <songliubraving@fb.com>
> > ---
> >  tools/perf/util/bpf-event.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> > index 01e1dc1bb7fb..f610872812ef 100644
> > --- a/tools/perf/util/bpf-event.c
> > +++ b/tools/perf/util/bpf-event.c
> > @@ -7,6 +7,7 @@
> >  #include "bpf-event.h"
> >  #include "debug.h"
> >  #include "symbol.h"
> > +#include "machine.h"
> >  
> >  #define ptr_to_u64(ptr)    ((__u64)(unsigned long)(ptr))
> >  
> > @@ -149,7 +150,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> >  		*ksymbol_event = (struct ksymbol_event){
> >  			.header = {
> >  				.type = PERF_RECORD_KSYMBOL,
> > -				.size = sizeof(struct ksymbol_event),
> > +				.size = offsetof(struct ksymbol_event, name),
> >  			},
> >  			.addr = prog_addrs[i],
> >  			.len = prog_lens[i],
> > @@ -178,6 +179,9 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> >  
> >  		ksymbol_event->header.size += PERF_ALIGN(name_len + 1,
> >  							 sizeof(u64));
> > +
> > +		memset(event + event->header.size, 0, machine->id_hdr_size);
> > +		event->header.size += machine->id_hdr_size;
> >  		err = perf_tool__process_synth_event(tool, event,
> >  						     machine, process);
> >  	}
> > @@ -194,6 +198,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
> >  			.id = info.id,
> >  		};
> >  		memcpy(bpf_event->tag, prog_tags[i], BPF_TAG_SIZE);
> > +		memset(event + event->header.size, 0, machine->id_hdr_size);
> > +		event->header.size += machine->id_hdr_size;

So, you need the patch below as well, the last bit isn't needed if your
ksymbol name is short, so one doesn't notice this when not applying that
last patch but it is needed:

Program received signal SIGSEGV, Segmentation fault.
__memset_avx2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:170
170		VMOVU	%VEC(0), -VEC_SIZE(%rdi,%rdx)
Missing separate debuginfos, use: dnf debuginfo-install bzip2-libs-1.0.6-28.fc29.x86_64 elfutils-libelf-0.174-5.fc29.x86_64 elfutils-libs-0.174-5.fc29.x86_64 glib2-2.58.2-1.fc29.x86_64 libbabeltrace-1.5.6-1.fc29.x86_64 libunwind-1.2.1-6.fc29.x86_64 libuuid-2.32.1-1.fc29.x86_64 libxcrypt-4.4.2-3.fc29.x86_64 numactl-libs-2.0.12-1.fc29.x86_64 openssl-libs-1.1.1a-1.fc29.x86_64 pcre-8.42-6.fc29.x86_64 perl-libs-5.28.1-427.fc29.x86_64 popt-1.16-15.fc29.x86_64 python2-libs-2.7.15-11.fc29.x86_64 slang-2.3.2-4.fc29.x86_64 xz-libs-5.2.4-3.fc29.x86_64
(gdb) bt
#0  __memset_avx2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memset-vec-unaligned-erms.S:170
#1  0x00000000005e0054 in perf_event__synthesize_one_bpf_prog (tool=0x9202a0 <record>, process=0x4451dc <process_synthesized_event>,
    machine=0xab67a0, fd=77, event=0xb4dd90, opts=0x9203b0 <record+272>) at util/bpf-event.c:183
#2  0x00000000005e03a3 in perf_event__synthesize_bpf_events (tool=0x9202a0 <record>, process=0x4451dc <process_synthesized_event>,
    machine=0xab67a0, opts=0x9203b0 <record+272>) at util/bpf-event.c:250
#3  0x0000000000446eac in record__synthesize (rec=0x9202a0 <record>, tail=false) at builtin-record.c:1086
#4  0x000000000044740f in __cmd_record (rec=0x9202a0 <record>, argc=2, argv=0xab4720) at builtin-record.c:1209
#5  0x0000000000448d64 in cmd_record (argc=2, argv=0xab4720) at builtin-record.c:2153
#6  0x00000000004406dd in __cmd_record (argc=3, argv=0x7fffffffdb30) at builtin-sched.c:3339
#7  0x00000000004411ff in cmd_sched (argc=3, argv=0x7fffffffdb30) at builtin-sched.c:3486
#8  0x00000000004d46e3 in run_builtin (p=0x932c50 <commands+432>, argc=4, argv=0x7fffffffdb30) at perf.c:302
#9  0x00000000004d4950 in handle_internal_command (argc=4, argv=0x7fffffffdb30) at perf.c:354
#10 0x00000000004d4a9f in run_argv (argcp=0x7fffffffd98c, argv=0x7fffffffd980) at perf.c:398
#11 0x00000000004d4e0b in main (argc=4, argv=0x7fffffffdb30) at perf.c:520
(gdb) fr 1
#1  0x00000000005e0054 in perf_event__synthesize_one_bpf_prog (tool=0x9202a0 <record>, process=0x4451dc <process_synthesized_event>,
    machine=0xab67a0, fd=77, event=0xb4dd90, opts=0x9203b0 <record+272>) at util/bpf-event.c:183
183			memset(event + event->header.size, 0, machine->id_hdr_size);
(gdb) p machine->id_hdr_size
$1 = 32
(gdb) p event
$2 = (union perf_event *) 0xb4dd90
(gdb) p event->header.size
$3 = 56

I'm folding the fixes below into your fix, thanks.

- Arnaldo

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index f610872812ef..b62c976e7c4c 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -180,7 +180,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
 		ksymbol_event->header.size += PERF_ALIGN(name_len + 1,
 							 sizeof(u64));
 
-		memset(event + event->header.size, 0, machine->id_hdr_size);
+		memset((void *)event + event->header.size, 0, machine->id_hdr_size);
 		event->header.size += machine->id_hdr_size;
 		err = perf_tool__process_synth_event(tool, event,
 						     machine, process);
@@ -198,7 +198,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
 			.id = info.id,
 		};
 		memcpy(bpf_event->tag, prog_tags[i], BPF_TAG_SIZE);
-		memset(event + event->header.size, 0, machine->id_hdr_size);
+		memset((void *)event + event->header.size, 0, machine->id_hdr_size);
 		event->header.size += machine->id_hdr_size;
 		err = perf_tool__process_synth_event(tool, event,
 						     machine, process);
@@ -223,7 +223,7 @@ int perf_event__synthesize_bpf_events(struct perf_tool *tool,
 	int err;
 	int fd;
 
-	event = malloc(sizeof(event->bpf_event) + KSYM_NAME_LEN);
+	event = malloc(sizeof(event->bpf_event) + KSYM_NAME_LEN + machine->id_idr_size);
 	if (!event)
 		return -1;
 	while (true) {

  reply	other threads:[~2019-01-24 10:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 21:02 [PATCH perf] perf tools: fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT Song Liu
2019-01-24  9:49 ` Arnaldo Carvalho de Melo
2019-01-24 10:29   ` Arnaldo Carvalho de Melo [this message]
2019-01-26 10:05 ` [tip:perf/core] perf bpf: Fix " tip-bot for Song Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190124102915.GA30768@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=jolsa@kernel.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=songliubraving@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).