From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752239AbdC2Cmc (ORCPT ); Tue, 28 Mar 2017 22:42:32 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:42638 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751963AbdC2Cmb (ORCPT ); Tue, 28 Mar 2017 22:42:31 -0400 X-Original-SENDERIP: 156.147.1.151 X-Original-MAILFROM: namhyung@kernel.org X-Original-SENDERIP: 10.177.227.17 X-Original-MAILFROM: namhyung@kernel.org Date: Wed, 29 Mar 2017 11:42:27 +0900 From: Namhyung Kim To: Steven Rostedt Cc: Ingo Molnar , Masami Hiramatsu , LKML , kernel-team@lge.com Subject: Re: [PATCH 1/4] ftrace: Fix function pid filter on instances Message-ID: <20170329024227.GC19448@sejong> References: <20170329014625.19346-1-namhyung@kernel.org> <20170329014625.19346-2-namhyung@kernel.org> <20170328220841.6823958d@grimm.local.home> <20170329022037.GA19448@sejong> <20170328222855.78af0d53@grimm.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170328222855.78af0d53@grimm.local.home> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Mar 28, 2017 at 10:28:55PM -0400, Steven Rostedt wrote: > On Wed, 29 Mar 2017 11:20:37 +0900 > Namhyung Kim wrote: > > > > > Actually, if this is called after event_trace_del_tracer(), the tr is > > > already invisible and nothing new should change. > > > > I don't follow. After event_trace_del_tracer(), the tr is invisible > > from the probe of event tracing but still is visible from the probe of > > function tracing, right? > > Well, nothing should be able to get to the set_ftrace_filter file when > there. Because of the tr->ref count. But keeping the lock is safer > regardless, and it's not a fast path, so the extra overhead if the lock > isn't needed is no big deal. Oh, I meant if a pid filter was already set when removing the instance. Function filters should be inactive since function tracer was finished (via tracing_set_nop), but the probe on sched_switch event (for pid filter) is still active and references the tr. Thanks, Namhyung > > > > > > > > > Make a wrapper around clear_ftrace_pids() and call that instead. We > > > don't even need to take a lock, but as I see there's a lockdep test for > > > ftrace_lock, we should still do so just to be safe. > > > > Right, that's why I call ftrace_pid_reset() instead of > > clear_ftrace_pids(). So do you prefer adding a new wrapper like below > > rather than reusing ftrace_pid_reset() with a new argument? > > Yes, because the bool passed in is confusing. A separate function like > below is more descriptive. > > -- Steve > > > > > Thanks, > > Namhyung > > > > > > > > > > void ftrace_clear_pids(struct trace_array *tr) > > > { > > > mutex_lock(&ftrace_lock); > > > > > > clear_ftrace_pids(tr); > > > > > > mutex_unlock(&ftrace_lock); > > > }