Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <f360d9c67bb9bc3418fad290399b8f27cb61c80f.camel@linux.intel.com>
Subject: Re: [RFC PATCH 0/5] tracing: Unifying dynamic event interface
From:   Tom Zanussi <tom.zanussi@linux.intel.com>
To:     Masami Hiramatsu <mhiramat@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>
Cc:     linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
        Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Date:   Mon, 01 Oct 2018 08:49:24 -0500
In-Reply-To: <153806391326.18458.3927382953014694981.stgit@devbox>
References: <153806391326.18458.3927382953014694981.stgit@devbox>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Masami,

On Fri, 2018-09-28 at 00:58 +0900, Masami Hiramatsu wrote:
> Hi,
> 
> This is an RFC series of unifying dynamic event interface on ftrace.
> Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes
> and synthetic. This series unifies kprobes and uprobes event
> interface on "dynamic_events". This enables us to add new dynamic
> events easily on same interface, e.g. function events.

This seems like a nice idea to me and I don't see any problems with the
patches themselves, so consider it

Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com>

> The older interfaces are left on the tracefs for backward
> compatibility at this moment.
> 
> dynamic_events syntax has no different from kprobe_events and
> uprobe_events. You can use same syntax for dynamic_events interface.
> 
> I think we can integrate synthetic events to this dynamic_events
> interface but it will requires new syntax. e.g.
> 
> echo "s:<event-name> <args>" >> dynamic_events
> 

So that's just the existing syntax, prefaced by s: , right?

> If it is OK, I'll add it in next version.
> 

Makes sense to me.

> BTW, since this dynamic_events interface derived from *probe_events,
> it inherits "all clear when truncate file open" behavior. But if you
> think this is too aggressive, I can drop it. (even in that case,
> kprobe_events/uprobe_events behavior is not changed)
> 
> I also introduced a widely used way to erase entries in other
> interfaces of ftrace, that is '!'. So you can now use '!event-name'
> or '!group/event' to erase an entry in dynamic_events.
> (Wait... it has to be '!p:event' as others do??)
> 

I'd think the full form should always be accepted, but would only be
necessary in cases requiring disambiguation.

Thanks,

Tom

> Any thought?
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (5):
>       tracing/uprobes: Add busy check when cleanup all uprobes
>       tracing: Add a unified dynamic event framework
>       tracing/kprobes: Use dyn_event framework for kprobe events
>       tracing/uprobes: Use dyn_event framework for uprobe events
>       tracing: Add generic event-name based remove event method
> 
> 
>  Documentation/trace/kprobetrace.rst  |    3 +
>  Documentation/trace/uprobetracer.rst |    4 +
>  kernel/trace/Kconfig                 |    4 +
>  kernel/trace/Makefile                |    1 
>  kernel/trace/trace.c                 |    4 +
>  kernel/trace/trace_dynevent.c        |  183
> +++++++++++++++++++++++++++++++++
>  kernel/trace/trace_dynevent.h        |   89 ++++++++++++++++
>  kernel/trace/trace_kprobe.c          |  191 +++++++++++++++++++++++-
> ----------
>  kernel/trace/trace_uprobe.c          |  175 +++++++++++++++++++++++-
> -------
>  9 files changed, 547 insertions(+), 107 deletions(-)
>  create mode 100644 kernel/trace/trace_dynevent.c
>  create mode 100644 kernel/trace/trace_dynevent.h
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>