Date: Wed, 8 Jul 2015 10:02:40 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: "Wangnan (F)" <wangnan0@huawei.com>
Cc: ast@plumgrid.com, brendan.d.gregg@gmail.com, daniel@iogearbox.net,
        Namhyung Kim <namhyung@kernel.org>,
        Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
        Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, David Ahern <dsahern@gmail.com>,
        linux-kernel@vger.kernel.org, lizefan@huawei.com, hekuang@huawei.com,
        xiakaixu@huawei.com, pi3orama@163.com
Subject: Re: [RFC PATCH v10 23/50] perf tools: Make perf depend on libbpf
Message-ID: <20150708130240.GA3243@kernel.org>
References: <1435716878-189507-1-git-send-email-wangnan0@huawei.com>
 <1435716878-189507-24-git-send-email-wangnan0@huawei.com>
 <20150707195452.GD3135@kernel.org>
 <20150707201656.GE3135@kernel.org>
 <559D0D5E.60901@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <559D0D5E.60901@huawei.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2827
Lines: 67

Em Wed, Jul 08, 2015 at 07:45:34PM +0800, Wangnan (F) escreveu:
> On 2015/7/8 4:16, Arnaldo Carvalho de Melo wrote:
> >Em Tue, Jul 07, 2015 at 04:54:52PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>Em Wed, Jul 01, 2015 at 02:14:11AM +0000, Wang Nan escreveu:
> >>>Error messages are also updated to notify users about the disable of
> >>>BPF support of 'perf record' if libelf is missed or BPF API check
> >>>failed.

> >>Much better!

> >But... I was all happy about this being linked with perf, went straight
> >ahead to try to use it! No, its not possible, I have to go thru a series
> >of other patches first... anticlimactic :-(

> >So, please move this to just before we can use it, wiring it up should
> >mean, hey, try this "hello, world" eBPF program right now!

> It is not an easy work, since there is a bulk of code in
> tools/perf/utils/bpf-loader.c depend on HAVE_LIBBPF_SUPPORT and
> CONFIG_LIBBPF. If put this patch the final one, we will make hundreds
> of lines of code avaiable by one patch.  It is not good.

Understood, makes sense, you are building the perf support step by step,
so it is not possible to test it in the first, only compile test the
initial wiring.
 
> I have an idea that, put this patch after the llvm tester:
 
> $ git log --oneline
> d011a28 perf tools: Make perf depend on libbpf
> 57ad12f perf tests: Add LLVM test for eBPF on-the-fly compiling
> 8c7e20b perf tools: Auto detecting kernel include options
> 442675f perf tools: Auto detecting kernel build directory
> dcd9304 perf tools: Call clang to compile C source to object code
> 864e2fb perf tools: Introduce llvm config options
> 8558c38 bpf tools: Link all bpf objects onto a list
> ...
 
> Then before this patch, you can test llvm on-the-fly compiling without
> parsing the
> result .o:
 
> $ perf test 38
> 38: Test LLVM searching and compiling                        : (skip bpf
> parsing) Ok
 
> After this patch, libbpf should be compiled so basic libbpf parsing can be
> tested:
> 
> $ perf test 38
> 38: Test LLVM searching and compiling                        : Ok

That looks enough for me, 'perf test' should exercise the code, making
sure that what was done up to that point got some testing.

We need to try to avoid adding too much code that doesn't get exercised
as soon as possible, otherwise we lose a lot of bisectability, i.e. when
trying to find a problem using 'git bisect' we will hit the first place
when all that code gets used and will be left with having to look at all
the code up to that point instead of landing in a more likely culprit.

- 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/