Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20171130014447.190229-9-songliubraving@fb.com>
References: <20171130014447.190229-1-songliubraving@fb.com> <20171130014447.190229-9-songliubraving@fb.com>
From:   Philippe Ombredanne <pombredanne@nexb.com>
Date:   Thu, 30 Nov 2017 09:24:18 +0100
Message-ID: <CAOFm3uHoPPM0yOUNpAMBsJTcxYwy7UTCbyPy2wOG33j3wsVw-Q@mail.gmail.com>
Subject: Re: [PATCH v2 6/6] bpf: add new test test_many_kprobe
To:     Song Liu <songliubraving@fb.com>
Cc:     peterz@infradead.org, rostedt@goodmis.org, mingo@redhat.com,
        "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>, daniel@iogearbox.net,
        kernel-team@fb.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Nov 30, 2017 at 2:44 AM, Song Liu <songliubraving@fb.com> wrote:
> The test compares old text based kprobe API with PERF_TYPE_KPROBE.
>
> Here is a sample output of this test:
>
> Creating 1000 kprobes with text-based API takes 6.979683 seconds
> Cleaning 1000 kprobes with text-based API takes 84.897687 seconds
> Creating 1000 kprobes with PERF_TYPE_KPROBE (function name) takes 5.077558 seconds
> Cleaning 1000 kprobes with PERF_TYPE_KPROBE (function name) takes 81.241354 seconds
> Creating 1000 kprobes with PERF_TYPE_KPROBE (function addr) takes 5.218255 seconds
> Cleaning 1000 kprobes with PERF_TYPE_KPROBE (function addr) takes 80.010731 seconds
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Reviewed-by: Josef Bacik <jbacik@fb.com>
> ---
>  samples/bpf/Makefile                |   3 +
>  samples/bpf/bpf_load.c              |   5 +-
>  samples/bpf/bpf_load.h              |   4 +
>  samples/bpf/test_many_kprobe_user.c | 182 ++++++++++++++++++++++++++++++++++++
>  4 files changed, 191 insertions(+), 3 deletions(-)
>  create mode 100644 samples/bpf/test_many_kprobe_user.c
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 9b4a66e..ec92f35 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -42,6 +42,7 @@ hostprogs-y += xdp_redirect
>  hostprogs-y += xdp_redirect_map
>  hostprogs-y += xdp_monitor
>  hostprogs-y += syscall_tp
> +hostprogs-y += test_many_kprobe
>
>  # Libbpf dependencies
>  LIBBPF := ../../tools/lib/bpf/bpf.o
> @@ -87,6 +88,7 @@ xdp_redirect-objs := bpf_load.o $(LIBBPF) xdp_redirect_user.o
>  xdp_redirect_map-objs := bpf_load.o $(LIBBPF) xdp_redirect_map_user.o
>  xdp_monitor-objs := bpf_load.o $(LIBBPF) xdp_monitor_user.o
>  syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
> +test_many_kprobe-objs := bpf_load.o $(LIBBPF) test_many_kprobe_user.o
>
>  # Tell kbuild to always build the programs
>  always := $(hostprogs-y)
> @@ -172,6 +174,7 @@ HOSTLOADLIBES_xdp_redirect += -lelf
>  HOSTLOADLIBES_xdp_redirect_map += -lelf
>  HOSTLOADLIBES_xdp_monitor += -lelf
>  HOSTLOADLIBES_syscall_tp += -lelf
> +HOSTLOADLIBES_test_many_kprobe += -lelf
>
>  # Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
>  #  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 872510e..caba9bc 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -635,9 +635,8 @@ void read_trace_pipe(void)
>         }
>  }
>
> -#define MAX_SYMS 300000
> -static struct ksym syms[MAX_SYMS];
> -static int sym_cnt;
> +struct ksym syms[MAX_SYMS];
> +int sym_cnt;
>
>  static int ksym_cmp(const void *p1, const void *p2)
>  {
> diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
> index e7a8a21..16bc263 100644
> --- a/samples/bpf/bpf_load.h
> +++ b/samples/bpf/bpf_load.h
> @@ -67,6 +67,10 @@ static inline __u64 ptr_to_u64(const void *ptr)
>         return (__u64) (unsigned long) ptr;
>  }
>
> +#define MAX_SYMS 300000
> +extern struct ksym syms[MAX_SYMS];
> +extern int sym_cnt;
> +
>  int load_kallsyms(void);
>  struct ksym *ksym_search(long key);
>  int set_link_xdp_fd(int ifindex, int fd, __u32 flags);
> diff --git a/samples/bpf/test_many_kprobe_user.c b/samples/bpf/test_many_kprobe_user.c
> new file mode 100644
> index 0000000..1f3ee07
> --- /dev/null
> +++ b/samples/bpf/test_many_kprobe_user.c
> @@ -0,0 +1,182 @@
> +/* Copyright (c) 2017 Facebook
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */

I think an SPDX id would be better here e.g. just this may be?

> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook

It should be on the first line as requested by Linus and documented by
Thomas (tglx) and Greg (greg-kh). And it should use // comments.
See threads on these topics.

-- 
Cordially
Philippe Ombredanne

From 1585453701316454613@xxx Thu Nov 30 01:47:54 +0000 2017
X-GM-THRID: 1585453701316454613
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread