Re: [PATCH v3 25/30] perf clang jit: Allow jitted perf hook access BPF maps

[Alexei Starovoitov]

On Sat, Nov 26, 2016 at 07:03:49AM +0000, Wang Nan wrote:
> Newly introduced jit-helpers.[ch] defines a series of helpers which helps
> jitted perf hook functions accessing BPF maps defined in their BPF scripts.
> The helpers fetches fd of 'struct bpf_map' from 'struct bpf_object' and the
> address of 'struct bpf_map_def' in jitted file. 'struct bpf_object' is the
> context passed to hooks.
> 
> Jit helpers added in this commits are all leading with 'perf_'. We don't use
> 'bpf_' prefix because in following commits 'bpf_' prefix is going to be assigned
> to kernel side BPF map operations. Same operation has different protocol for
> kernel and user.
> 
>  Example:
> 
>   $ cat ./test.c
>   /*******************************************************/
>   #define SEC(name) __attribute__((section(name), used))
>   #define BPF_MAP_TYPE_ARRAY 2
>   #define BPF_MAP_TYPE_PERF_EVENT_ARRAY 4
>   #define BPF_FUNC_map_lookup_elem 1
>   static void *(*bpf_map_lookup_elem)(void *map, void *key) =
>       (void *) BPF_FUNC_map_lookup_elem;
>   struct bpf_map_def {
>       unsigned int type;
>       unsigned int key_size;
>       unsigned int value_size;
>       unsigned int max_entries;
>   };
>   struct bpf_map_def SEC("maps") counter = {
>       .type = BPF_MAP_TYPE_ARRAY,
>       .key_size = sizeof(int),
>       .value_size = sizeof(int),
>       .max_entries = 1,
>   };
>   extern int perf_map_update_elem(void *ctx, struct bpf_map_def *map,
>                          void *key, void *value, unsigned long flags);
>   extern int perf_map_lookup_elem(void *ctx, struct bpf_map_def *map,
>                          void *key, void *value);
>   SEC("sys_close=SyS_close")
>   int sys_close(void *ctx)
>   {
>       int key = 0;
>       int *value;
>       value = bpf_map_lookup_elem(&counter, &key);
>       if (!value)
>           return 0;
>       __sync_fetch_and_add(value, 1);
>       return 0;
>   }
>   extern int printf(const char *fmt, ...);
>   SEC("perfhook:record_start")
>   void record_start(void *ctx)
>   {
>       int key = 0;
>       int value = 100000000;
>       printf("Welcom to perf record\n");
>       perf_map_update_elem(ctx, &counter, &key, &value, 0);
>   }
> 
>   SEC("perfhook:record_end")
>   void record_end(void *ctx)
>   {
>       int key = 0;
>       int value;
>       perf_map_lookup_elem(ctx, &counter, &key, &value);
>       printf("Goodbye, perf record, value=%d\n", value);
>   }
>   char _license[] SEC("license") = "GPL";
>   int _version SEC("version") = LINUX_VERSION_CODE;
>   /*******************************************************/
>   $ sudo perf record  -e ./test.c echo Hehe
>   Welcom to perf record
>   Hehe
>   [ perf record: Woken up 1 times to write data ]
>   Goodbye, perf record, value=100000644
>   [ perf record: Captured and wrote 0.014 MB perf.data ]
> 
> Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
...
> +#define __stringify_1(x)	#x
> +#define __stringify(x)		__stringify_1(x)
>  static std::map<const std::string, const void *> exported_funcs =
>  {
> -#define EXPORT(f) {#f, (const void *)&f}
> +#define EXPORT(f) {__stringify(f), (const void *)&f}
>  	EXPORT(test__clang_callback),
>  	EXPORT(printf),
>  	EXPORT(puts),
> +	EXPORT(JIT_HELPER_FUNC_NAME(map_update_elem)),
> +	EXPORT(JIT_HELPER_FUNC_NAME(map_lookup_elem)),
> +	EXPORT(JIT_HELPER_FUNC_NAME(map_get_next_key)),
> +	EXPORT(JIT_HELPER_FUNC_NAME(map_pin)),
>  #undef EXPORT
...
> +#define PARAMS(args...) args
> +#define DEFINE_JIT_BPF_MAP_HELPER(name, proto, args)			\
> +	JIT_BPF_MAP_HELPER(name, proto) {				\
> +		int map_fd = get_bpf_map_fd(ctx, map);			\
> +									\
> +		if (map_fd < 0)						\
> +			return map_fd;					\
> +		return bpf_map_##name(map_fd, args);			\
> +	}
> +
> +DEFINE_JIT_BPF_MAP_HELPER(update_elem,
> +			  PARAMS(void *key, void *value, u64 flags),
> +			  PARAMS(key, value, flags))

the naming and approach to exports look good to me.
Acked-by: Alexei Starovoitov <ast@xxxxxxxxxx>