Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752022AbcKOFVs (ORCPT ); Tue, 15 Nov 2016 00:21:48 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35376 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754198AbcKOFVr (ORCPT ); Tue, 15 Nov 2016 00:21:47 -0500 MIME-Version: 1.0 From: Alexei Starovoitov Date: Mon, 14 Nov 2016 21:21:25 -0800 Message-ID: Subject: Re: [PATCH 00/34] perf clang: Builtin clang and perfhook support To: "Wangnan (F)" Cc: Arnaldo Carvalho de Melo , Alexei Starovoitov , Zefan Li , He Kuang , "linux-kernel@vger.kernel.org" , pi3orama , Jiri Olsa Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1628 Lines: 45 On Mon, Nov 14, 2016 at 9:03 PM, Wangnan (F) wrote: > > > On 2016/11/15 12:57, Alexei Starovoitov wrote: >> >> On Mon, Nov 14, 2016 at 8:05 PM, Wang Nan wrote: >>> >>> This is version 2 of perf builtin clang patch series. Compare to v1, >>> add an exciting feature: jit compiling perf hook functions. This >>> features allows script writer report result through BPF map in a >>> customized way. >> >> looks great. >> >>> SEC("perfhook:record_start") >>> void record_start(void *ctx) >>> { >>> int perf_pid = getpid(), key = G_perf_pid; >>> printf("Start count, perfpid=%d\n", perf_pid); >>> jit_helper__map_update_elem(ctx, &GVALS, &key, &perf_pid, 0); >> >> the name, I think, is too verbose. >> Why not to keep them as bpf_map_update_elem >> even for user space programs? > > > I can make it shorter by give it a better name or use a wrapper like > > BPF_MAP(update_elem) the macro isn't pretty, since function calls won't look like calls. > but the only thing I can't do is to make perfhook and in-kernel script > use a uniform name for these bpf_map functions, because > bpf_map_update_elem is already defined: > > "static long (*bpf_map_update_elem)(void *, void *, void *, unsigned long) = > (void *)2;\n" right. i guess you could have #ifdef it, so it's different for bpf backend and for native. Another alternative is to call it map_update_elem or map_update or bpf_map_update. Something shorter is already a win. 'jit_helper__' prefix is an implementation detail. The users don't need to know and don't need to spell it out everywhere.