>From: Namhyung Kim [mailto:namhy...@gmail.com] On Behalf Of Namhyung Kim > >On Sun, Sep 06, 2015 at 03:47:37PM +0800, Wangnan (F) wrote: >> Hi Namhyung, > >Hi, > >> >> 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
This is for clearing an array of probe events. >> del_perf_probe_events This is not for cleanup, but for removing probes in the kernel. >> clear_perf_probe_event >> clear_probe_trace_event These are the cleanup each event. Ah, right, since now perf_probe_event has probe_trace_events, clear_perf_probe_event has to call clear_probe_trace_event. >> >> But non of them works perfectly for me. > >The cleanup_perf_probe_events() is just to keep the existing logic as >long as possible. But I think it needs to call >clear_perf_probe_event(). > >The del_perf_probe_events() uses strfilter, but I think it can be >problematic if other instances or users are using similar events at >the same time. Yeah, since perf probe doesn't lock the ftrace, there should be a timing bug, but it can be fixed easily by ignoring -ENOENT. :) >So for your case, IMHO it'd better keeping the perf/trace events after >probing and reusing the events for unprobing. I'll take a look at it. > > >> >> 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. > >Yeah, I also think exit_symbol_maps() should not be a part of the >cleanup. I'll send a patch soon. OK. Thanks!
