lkml.org 
[lkml]   [2015]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 1/5] perf probe: Split add_perf_probe_events()
    On Sun, Sep 06, 2015 at 03:47:37PM +0800, Wangnan (F) wrote:
    > Hi Namhyung,

    Hi,

    I'm off until Wednesday. I'll be able to take a look at it on
    Thursday.

    Thanks,
    Namhyung


    >
    > Thanks for this patchset.
    >
    > Could you plase have a look at patch 5/27 and 6/27 in my newest pull
    > request?
    > These 2 patches utilize new probing API to create probe point and collect
    > probe_trace_events. I'm not very sure I fully understand your design
    > principle,
    > especially the cleanup part, because I can see different functions dealing
    > with
    > cleanup:
    >
    > cleanup_perf_probe_events
    > del_perf_probe_events
    > clear_perf_probe_event
    > clear_probe_trace_event
    >
    > But non of them works perfectly for me.
    >
    > In bpf_prog_priv__clear() function of 6/27, I copied some code from
    > cleanup_perf_probe_events(), because I think when destroying bpf programs,
    > the probe_trace_events should also be cleanuped, but we don't need call
    > exit_symbol_maps() many times, because we are in 'perf record', and not
    > sure whether other parts of perf need symbol maps. Otherwise I think
    > directly
    > calling cleanup_perf_probe_events() sould be better.
    >
    > You can find patch from:
    >
    > http://lkml.kernel.org/n/1441523623-152703-6-git-send-email-wangnan0@huawei.com
    >
    > http://lkml.kernel.org/n/1441523623-152703-7-git-send-email-wangnan0@huawei.com
    >
    > Thank you.
    >
    > On 2015/9/4 20:15, Namhyung Kim wrote:
    > >The add_perf_probe_events() does 3 things:
    > >
    > > 1. convert all perf events to trace events
    > > 2. add all trace events to kernel
    > > 3. cleanup all trace events
    > >
    > >But sometimes we need to do something with the trace events. So split
    > >the funtion into three, so that it can access intermediate trace events
    > >via struct __event_package if needed.
    > >
    > >Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    > >Signed-off-by: Namhyung Kim <namhyung@kernel.org>
    > >---
    > > tools/perf/util/probe-event.c | 39 +++++++++++++++++++++++++++++++++++----
    > > 1 file changed, 35 insertions(+), 4 deletions(-)
    > >
    > >diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
    > >index eb5f18b75402..2c762f41e7a5 100644
    > >--- a/tools/perf/util/probe-event.c
    > >+++ b/tools/perf/util/probe-event.c
    > >@@ -2765,9 +2765,10 @@ struct __event_package {
    > > int ntevs;
    > > };
    > >-int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
    > >+static int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs,
    > >+ struct __event_package **ppkgs)
    > > {
    > >- int i, j, ret;
    > >+ int i, ret;
    > > struct __event_package *pkgs;
    > > ret = 0;
    > >@@ -2792,12 +2793,21 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
    > > ret = convert_to_probe_trace_events(pkgs[i].pev,
    > > &pkgs[i].tevs);
    > > if (ret < 0)
    > >- goto end;
    > >+ return ret;
    > > pkgs[i].ntevs = ret;
    > > }
    > > /* This just release blacklist only if allocated */
    > > kprobe_blacklist__release();
    > >+ *ppkgs = pkgs;
    > >+
    > >+ return 0;
    > >+}
    > >+
    > >+static int apply_perf_probe_events(struct __event_package *pkgs, int npevs)
    > >+{
    > >+ int i, ret = 0;
    > >+
    > > /* Loop 2: add all events */
    > > for (i = 0; i < npevs; i++) {
    > > ret = __add_probe_trace_events(pkgs[i].pev, pkgs[i].tevs,
    > >@@ -2806,7 +2816,16 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
    > > if (ret < 0)
    > > break;
    > > }
    > >-end:
    > >+ return ret;
    > >+}
    > >+
    > >+static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
    > >+{
    > >+ int i, j;
    > >+
    > >+ if (pkgs == NULL)
    > >+ return;
    > >+
    > > /* Loop 3: cleanup and free trace events */
    > > for (i = 0; i < npevs; i++) {
    > > for (j = 0; j < pkgs[i].ntevs; j++)
    > >@@ -2815,6 +2834,18 @@ end:
    > > }
    > > free(pkgs);
    > > exit_symbol_maps();
    > >+}
    > >+
    > >+int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
    > >+{
    > >+ int ret;
    > >+ struct __event_package *pkgs = NULL;
    > >+
    > >+ ret = convert_perf_probe_events(pevs, npevs, &pkgs);
    > >+ if (ret == 0)
    > >+ ret = apply_perf_probe_events(pkgs, npevs);
    > >+
    > >+ cleanup_perf_probe_events(pkgs, npevs);
    > > return ret;
    > > }
    >
    >


    \
     
     \ /
      Last update: 2015-09-08 04:21    [W:4.669 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site