From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753346AbbIJFAR (ORCPT ); Thu, 10 Sep 2015 01:00:17 -0400 Received: from mail4.hitachi.co.jp ([133.145.228.5]:42880 "EHLO mail4.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750831AbbIJFAM (ORCPT ); Thu, 10 Sep 2015 01:00:12 -0400 From: =?utf-8?B?5bmz5p2+6ZuF5bezIC8gSElSQU1BVFXvvIxNQVNBTUk=?= To: "'Namhyung Kim'" , "Wangnan (F)" CC: Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , "pi3orama@163.com" Subject: RE: Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Thread-Topic: [!!]Re: [PATCH v2 1/5] perf probe: Split add_perf_probe_events() Thread-Index: AQHQ63AhyxIk485b4U2EfniKQxuy+Z41Mkcw Date: Thu, 10 Sep 2015 05:00:07 +0000 Message-ID: <50399556C9727B4D88A595C8584AAB375252011D@GSjpTKYDCembx32.service.hitachi.net> References: <1441368963-11565-1-git-send-email-namhyung@kernel.org> <55EBEF99.8010506@huawei.com> <20150910022325.GA32720@danjae.kornet> In-Reply-To: <20150910022325.GA32720@danjae.kornet> Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.198.219.44] Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id t8A50MBd031151 >From: Namhyung Kim [mailto:namhyung@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! {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I