Date: Fri, 7 Aug 2015 10:43:38 -0300
From: Arnaldo Carvalho de Melo <acme@redhat.com>
To: "Wangnan (F)" <wangnan0@huawei.com>
Cc: ast@plumgrid.com, daniel@iogearbox.net, David Ahern <dsahern@gmail.com>,
        hekuang@huawei.com, Jiri Olsa <jolsa@kernel.org>, xiakaixu@huawei.com,
        masami.hiramatsu.pt@hitachi.com, Namhyung Kim <namhyung@gmail.com>,
        a.p.zijlstra@chello.nl, lizefan@huawei.com, pi3orama@163.com,
        acme@kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 09/20] perf tools: Parse probe points of eBPF programs
 during preparation
Message-ID: <20150807134338.GB2203@redhat.com>
References: <1438669729-1860-1-git-send-email-wangnan0@huawei.com>
 <1438669729-1860-10-git-send-email-wangnan0@huawei.com>
 <20150806155000.GD2265@redhat.com>
 <55C441CB.5020509@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <55C441CB.5020509@huawei.com>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3091
Lines: 71

Adding back lkml and Ingo, noticed it wasn't there :-\

Em Fri, Aug 07, 2015 at 01:27:39PM +0800, Wangnan (F) escreveu:
> On 2015/8/6 23:50, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Aug 04, 2015 at 06:28:38AM +0000, Wang Nan escreveu:
> >>This patch parses section name of each program, and creates
> >>corresponding 'struct perf_probe_event' structure.
> >>
> >>parse_perf_probe_command() is used to do the main parsing works.
> >>Parsing result is stored into a global array. This is because
> >>add_perf_probe_events() is non-reentrantable. In following patch,
> >>add_perf_probe_events will be introduced to insert kprobes. It accepts
> >>an array of 'struct perf_probe_event' and do all works in one call.
> >>
> >>Define PERF_BPF_PROBE_GROUP as "perf_bpf_probe", which will be used
> >>as group name of all eBPF probing points.
> >>
> >>This patch utilizes bpf_program__set_private(), bind perf_probe_event
> >>with bpf program by private field.
> >This patch reports errors via pr_err(), which is suboptimal because it
> >assumes that the interface to these facilities use a stdout UI.
> 
> Can I fix it using one patch after all staffs in
> tools/perf/utils/bpf-loader.c
> goes into your tree?

Sure
 
> >I guess we can have it as is now, because the initial use cases is stdio
> >based, but we need to move this to use bpf__strerror() routines like we
> >do elsewhere in tools/perf/, i.e. see:
> >
> >int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused,
> >                                int err, char *buf, size_t size)
> >
> >int perf_evlist__strerror_mmap(struct perf_evlist *evlist, int err,
> >			       char *buf, size_t size)
> >
> >I.e. we have multiple CLASS__strerror_API to map errors we return (that
> >int err) to strings that we store in buf, following the strerror_r()
> >model.
> >
> >I.e. these libraries should not emit errors, at most they can emit log
> >messages (pr_debug), but direct user interaction should be done via UI
> >code (tools/perf/ui/) and tools/perf/builtin-foo.c files.
> >
> >Also the global thing suckz rockz, again, may be acceptable at this
> >point due to friction with how the existing probing routines are
> >structured, but we need to make those functions better, not work around
> >what seems to be limitations they have.
> 
> Also for this one, since it is a fundamental problem in probe related
> code. However, I think global thing can be avoided by moving this patch
> after 'perf probe: Attach trace_probe_event with perf_probe_event',
> config programs at the head of bpf__probe().

Right, I'm preparing a pull req for up to the 'perf test LLVM' patch,
which I think we went thru enough iterations, that way we erode this
patchkit a bit more, as 4 patches are already in tip.git, from
yesterday's pull req I sent to Ingo, yay!

> Thank you.

Thanks!

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/