From: Sumanth Korikkar <sumanthk@linux.ibm.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org, bpf@vger.kernel.org,
jolsa@redhat.com, tmricht@linux.ibm.com,
heiko.carstens@de.ibm.com, mhiramat@kernel.org,
iii@linux.ibm.com
Subject: Re: [PATCH] perf: Fix bpf prologue generation, user attribute access in kprobes
Date: Tue, 9 Jun 2020 09:00:47 +0200 [thread overview]
Message-ID: <fb0e9318-a905-0a77-524b-f6f16d670c22@linux.ibm.com> (raw)
In-Reply-To: <20200608164327.GD3073@kernel.org>
Hi Arnaldo,
On 6/8/20 6:43 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 04, 2020 at 11:10:28AM +0200, Sumanth Korikkar escreveu:
>> Issues:
>> 1. bpf_probe_read is no longer available for architecture which has
>> overlapping address space. Hence bpf prologue generation fails
>> 2. perf probe -a 'do_sched_setscheduler pid policy
>> param->sched_priority@user' did not work before.
> This looks super nice, thanks for working on this!
>
> But please split this into multiple patches, one for the 'perf probe'
> part, add the Acked-by provided by Masami to that one, and the other for
> the part fixing the lack of bpf_probe_read(), ok?
Thank you. Ok sure. I will send the split patches.
Best Regards
Sumanth
>
> Thanks!
>
> - Arnaldo
>
>> Fixes:
>> 1. Use bpf_probe_read_kernel for kernel member access. For user
>> attribute access in kprobes, bpf_probe_read_user is utilized
>> 2. Make (perf probe -a 'do_sched_setscheduler pid policy
>> param->sched_priority@user') output equivalent to ftrace
>> ('p:probe/do_sched_setscheduler _text+517384 pid=%r2:s32
>> policy=%r3:s32 sched_priority=+u0 (%r4):s32' > kprobe_events)
>>
>> Other:
>> 1. Right now, __match_glob() does not handle [u]<offset>. For now, use
>> *u]<offset>.
>> 2. @user attribute was introduced in commit 1e032f7cfa14 ("perf-probe:
>> Add user memory access attribute support")
>>
>> Test:
>> 1. ulimit -l 128 ; ./perf record -e tests/bpf_sched_setscheduler.c
>> 2. cat tests/bpf_sched_setscheduler.c
>>
>> static void (*bpf_trace_printk)(const char *fmt, int fmt_size, ...) =
>> (void *) 6;
>> static int (*bpf_probe_read_user)(void *dst, __u32 size,
>> const void *unsafe_ptr) = (void *) 112;
>> static int (*bpf_probe_read_kernel)(void *dst, __u32 size,
>> const void *unsafe_ptr) = (void *) 113;
>>
>> SEC("func=do_sched_setscheduler pid policy param->sched_priority@user")
>> int bpf_func__setscheduler(void *ctx, int err, pid_t pid, int policy,
>> int param)
>> {
>> char fmt[] = "prio: %ld";
>> bpf_trace_printk(fmt, sizeof(fmt), param);
>> return 1;
>> }
>>
>> char _license[] SEC("license") = "GPL";
>> int _version SEC("version") = LINUX_VERSION_CODE;
>>
>> 3. ./perf script
>> sched 305669 [000] 1614458.838675: perf_bpf_probe:func: (2904e508)
>> pid=261614 policy=2 sched_priority=1
>>
>> 4. cat /sys/kernel/debug/tracing/trace
>> <...>-309956 [006] .... 1616098.093957: 0: prio: 1
>>
>> 5. bpf_probe_read_user is used when @user attribute is used. Otherwise,
>> defaults to bpf_probe_read_kernel
>>
>> 6. ./perf test bpf (null_lseek - kernel member access)
>> 42: BPF filter :
>> 42.1: Basic BPF filtering : Ok
>> 42.2: BPF pinning : Ok
>> 42.3: BPF prologue generation : Ok
>> 42.4: BPF relocation checker : FAILED!
>>
>> Signed-off-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
>> Reviewed-by: Thomas Richter <tmricht@linux.ibm.com>
>> ---
>> tools/perf/util/bpf-prologue.c | 14 ++++++++++----
>> tools/perf/util/probe-event.c | 7 +++++--
>> tools/perf/util/probe-file.c | 2 +-
>> 3 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/perf/util/bpf-prologue.c b/tools/perf/util/bpf-prologue.c
>> index b020a8678eb9..9887ae09242d 100644
>> --- a/tools/perf/util/bpf-prologue.c
>> +++ b/tools/perf/util/bpf-prologue.c
>> @@ -142,7 +142,8 @@ static int
>> gen_read_mem(struct bpf_insn_pos *pos,
>> int src_base_addr_reg,
>> int dst_addr_reg,
>> - long offset)
>> + long offset,
>> + int probeid)
>> {
>> /* mov arg3, src_base_addr_reg */
>> if (src_base_addr_reg != BPF_REG_ARG3)
>> @@ -159,7 +160,7 @@ gen_read_mem(struct bpf_insn_pos *pos,
>> ins(BPF_MOV64_REG(BPF_REG_ARG1, dst_addr_reg), pos);
>>
>> /* Call probe_read */
>> - ins(BPF_EMIT_CALL(BPF_FUNC_probe_read), pos);
>> + ins(BPF_EMIT_CALL(probeid), pos);
>> /*
>> * Error processing: if read fail, goto error code,
>> * will be relocated. Target should be the start of
>> @@ -241,7 +242,7 @@ static int
>> gen_prologue_slowpath(struct bpf_insn_pos *pos,
>> struct probe_trace_arg *args, int nargs)
>> {
>> - int err, i;
>> + int err, i, probeid;
>>
>> for (i = 0; i < nargs; i++) {
>> struct probe_trace_arg *arg = &args[i];
>> @@ -276,11 +277,16 @@ gen_prologue_slowpath(struct bpf_insn_pos *pos,
>> stack_offset), pos);
>>
>> ref = arg->ref;
>> + probeid = BPF_FUNC_probe_read_kernel;
>> while (ref) {
>> pr_debug("prologue: arg %d: offset %ld\n",
>> i, ref->offset);
>> +
>> + if (ref->user_access)
>> + probeid = BPF_FUNC_probe_read_user;
>> +
>> err = gen_read_mem(pos, BPF_REG_3, BPF_REG_7,
>> - ref->offset);
>> + ref->offset, probeid);
>> if (err) {
>> pr_err("prologue: failed to generate probe_read function call\n");
>> goto errout;
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index eea132f512b0..7cdb66ef3baa 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1565,7 +1565,7 @@ static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
>> }
>>
>> tmp = strchr(str, '@');
>> - if (tmp && tmp != str && strcmp(tmp + 1, "user")) { /* user attr */
>> + if (tmp && tmp != str && !strcmp(tmp + 1, "user")) { /* user attr */
>> if (!user_access_is_supported()) {
>> semantic_error("ftrace does not support user access\n");
>> return -EINVAL;
>> @@ -1986,7 +1986,10 @@ static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
>> if (depth < 0)
>> return depth;
>> }
>> - err = strbuf_addf(buf, "%+ld(", ref->offset);
>> + if (ref->user_access)
>> + err = strbuf_addf(buf, "%s%ld(", "+u", ref->offset);
>> + else
>> + err = strbuf_addf(buf, "%+ld(", ref->offset);
>> return (err < 0) ? err : depth;
>> }
>>
>> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
>> index 8c852948513e..064b63a6a3f3 100644
>> --- a/tools/perf/util/probe-file.c
>> +++ b/tools/perf/util/probe-file.c
>> @@ -1044,7 +1044,7 @@ static struct {
>> DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
>> DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
>> DEFINE_TYPE(FTRACE_README_UPROBE_REF_CTR, "*ref_ctr_offset*"),
>> - DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*[u]<offset>*"),
>> + DEFINE_TYPE(FTRACE_README_USER_ACCESS, "*u]<offset>*"),
>> DEFINE_TYPE(FTRACE_README_MULTIPROBE_EVENT, "*Create/append/*"),
>> DEFINE_TYPE(FTRACE_README_IMMEDIATE_VALUE, "*\\imm-value,*"),
>> };
>> --
>> 2.25.4
>>
--
Sumanth Korikkar
prev parent reply other threads:[~2020-06-09 7:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-04 9:10 [PATCH] perf: Fix bpf prologue generation, user attribute access in kprobes Sumanth Korikkar
2020-06-04 15:16 ` Masami Hiramatsu
2020-06-08 16:43 ` Arnaldo Carvalho de Melo
2020-06-09 7:00 ` Sumanth Korikkar [this message]
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=fb0e9318-a905-0a77-524b-f6f16d670c22@linux.ibm.com \
--to=sumanthk@linux.ibm.com \
--cc=acme@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=heiko.carstens@de.ibm.com \
--cc=iii@linux.ibm.com \
--cc=jolsa@redhat.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=tmricht@linux.ibm.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).