LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Tom Zanussi <tom.zanussi@linux.intel.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Subject: Re: [PATCH v2 00/12] tracing: Unifying dynamic event interface
Date: Wed, 28 Nov 2018 17:42:22 -0600
Message-ID: <ff8ca3062cad3747a664e6888790cc3c58e24c7e.camel@linux.intel.com> (raw)
In-Reply-To: <20181128163101.d86844338bcba3fc3b0580f1@kernel.org>

Hi Masami,

On Wed, 2018-11-28 at 16:31 +0900, Masami Hiramatsu wrote:
> Ping?
> 
> Hi Tom,
> 
> This series, especially [09/12] tracing: Remove unneeded
> synth_event_mutex
> will effect your current working series. Please tell me your opinion.
> 

Sorry for the delay in reviewing this - I completely forgot about it in
my inbox.

It's all very nice, and the mutex updates along with the dyn_event
management make that part of the code much cleaner, thanks!

In any case, you can have my

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

Thanks,

Tom


> Thank you,
> 
> On Mon,  5 Nov 2018 17:59:46 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Hi,
> > 
> > This is v2 series of unifying dynamic event interface on ftrace.
> > Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes
> > and synthetic. This series unifies those dynamic event interfaces
> > to "dynamic_events" so that we can add other dynamic events easily
> > on same interface, e.g. function events.
> > The older interfaces are left on the tracefs for backward
> > compatibility.
> > 
> > dynamic_events syntax has no difference from kprobe_events and
> > uprobe_events. You can use same syntax for dynamic_events
> > interface.
> > For synthetic events, similar to the probe events, dynamic_events
> > adds "s:[GROUP/]" prefix, where the "GROUP/" must be "synthetic/".
> > 
> >  s:[synthetic/]<event-name> type arg [type arg]...
> > 
> > E.g.
> > 
> >  $ echo 'wakeup_latency u64 lat pid_t pid char' > synthetic_events
> > 
> > is same as
> > 
> >  $ echo 's:wakeup_latency u64 lat pid_t pid char' > dynamic_events
> > 
> > Or
> > 
> >  $ echo 's:synthetic/wakeup_latency u64 lat pid_t pid char' >
> > dynamic_events
> > 
> > This series modifies synthetic event interface behavior a bit,
> > reorder lock dependency and related cleanups so that we can
> > integrate
> > the synthetic event to dynamic_events interface. 
> > 
> > In this version, I changed the generic '!' erase command, which
> > now supports entire line style like other interfaces. So you can
> > delete events via dynamic_events as below
> > 
> >  $ cat dynamic_events | while read line; \
> >    do echo "!$line" >> dynamic_events; done
> > 
> > Also, the big change will be removing dyn_event_mutex and
> > synth_event_mutex because all those parts are protected by
> > event_mutex.
> > 
> > Changes from v2 are here;
> > 
> > New patches:
> >  - Reorder event_mutex and synth_event_mutex to solve
> >    AB-BA deadlock correctly. ([2/12])
> >  - Simplify creation and deletion of synthetic event. ([3/12])
> >  - Retern -ENOENT if there is no synthetic event when deleting
> > ([4/12])
> >  - Integrate similar probe argument parsers ([5/12])
> >  - Use dyn_event framework for synthetic events ([9/12])
> >  - Remove synth_event_mutex ([10/12])
> >  - Remove unused APIs ([11/12])
> > 
> > Modified patches:
> >  [6/12] - [8/12]
> >  - Generalize delete event and export as dyn_event_release_all().
> >  - Add match operation for find deleting event.
> >  - Reorder event_mutex and dyn_event_mutex to solve lock dependency
> >    issue.
> >  - Pass const char **argv for create operation and use -ECANCELED
> > to
> >    signal for trying next dyn_event_operations.
> >  - Remove dyn_event_mutex.
> > 
> >  [12/12]
> >  - Accept entire line, but instead of checking the given entire
> > line
> >    strictly, simply checking the event and group name.
> > 
> > Tom, thanks for your Ack for v1 series. Since I changed many things
> > from v1 (not only minor change), I decided to not add your Ack for
> > this version. Anyway, what I've added in this version are related
> > to
> > synthetic events. I need your review for those.
> > (especially removing synth_event_mutex)
> > 
> > You can try it from my git tree.
> > 
> >   https://github.com/mhiramat/linux/tree/unify-dynamic-events-v2
> > 
> > Thank you,
> > 
> > ---
> > 
> > Masami Hiramatsu (12):
> >       tracing/uprobes: Add busy check when cleanup all uprobes
> >       tracing: Lock event_mutex before synth_event_mutex
> >       tracing: Simplify creation and deletion of synthetic event
> >       tracing: Integrate similar probe argument parsers
> >       tracing: Add unified dynamic event framework
> >       tracing/kprobes: Use dyn_event framework for kprobe events
> >       tracing/uprobes: Use dyn_event framework for uprobe events
> >       tracing: Use dyn_event framework for synthetic events
> >       tracing: Remove unneeded synth_event_mutex
> >       tracing: Remove orphaned trace_add/remove_event_call
> > functions
> >       tracing: Add generic event-name based remove event method
> >       selftests/ftrace: Add testcases for dynamic event
> > 
> > 
> >  Documentation/trace/kprobetrace.rst                |    3 
> >  Documentation/trace/uprobetracer.rst               |    4 
> >  include/linux/trace_events.h                       |    4 
> >  kernel/trace/Kconfig                               |    6 
> >  kernel/trace/Makefile                              |    1 
> >  kernel/trace/trace.c                               |   12 +
> >  kernel/trace/trace_dynevent.c                      |  217
> > ++++++++++++
> >  kernel/trace/trace_dynevent.h                      |  119 +++++++
> >  kernel/trace/trace_events.c                        |   12 -
> >  kernel/trace/trace_events_hist.c                   |  322
> > ++++++++++--------
> >  kernel/trace/trace_kprobe.c                        |  357
> > ++++++++++----------
> >  kernel/trace/trace_probe.c                         |   74 ++++
> >  kernel/trace/trace_probe.h                         |    9 -
> >  kernel/trace/trace_uprobe.c                        |  305
> > ++++++++---------
> >  .../ftrace/test.d/dynevent/add_remove_kprobe.tc    |   30 ++
> >  .../ftrace/test.d/dynevent/add_remove_synth.tc     |   27 ++
> >  .../ftrace/test.d/dynevent/clear_select_events.tc  |   50 +++
> >  .../ftrace/test.d/dynevent/generic_clear_event.tc  |   49 +++
> >  18 files changed, 1094 insertions(+), 507 deletions(-)
> >  create mode 100644 kernel/trace/trace_dynevent.c
> >  create mode 100644 kernel/trace/trace_dynevent.h
> >  create mode 100644
> > tools/testing/selftests/ftrace/test.d/dynevent/add_remove_kprobe.tc
> >  create mode 100644
> > tools/testing/selftests/ftrace/test.d/dynevent/add_remove_synth.tc
> >  create mode 100644
> > tools/testing/selftests/ftrace/test.d/dynevent/clear_select_events.
> > tc
> >  create mode 100644
> > tools/testing/selftests/ftrace/test.d/dynevent/generic_clear_event.
> > tc
> > 
> > --
> > Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
> 
> 


  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-05  8:59 Masami Hiramatsu
2018-11-05  9:00 ` [PATCH v2 01/12] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu
2018-12-04 17:43   ` Steven Rostedt
2018-12-07  2:19     ` Masami Hiramatsu
2018-11-05  9:00 ` [PATCH v2 02/12] tracing: Lock event_mutex before synth_event_mutex Masami Hiramatsu
2018-11-05  9:01 ` [PATCH v2 03/12] tracing: Simplify creation and deletion of synthetic event Masami Hiramatsu
2018-11-05  9:01 ` [PATCH v2 04/12] tracing: Integrate similar probe argument parsers Masami Hiramatsu
2018-11-05  9:02 ` [PATCH v2 05/12] tracing: Add unified dynamic event framework Masami Hiramatsu
2018-11-05  9:02 ` [PATCH v2 06/12] tracing/kprobes: Use dyn_event framework for kprobe events Masami Hiramatsu
2018-11-05  9:03 ` [PATCH v2 07/12] tracing/uprobes: Use dyn_event framework for uprobe events Masami Hiramatsu
2018-11-05  9:03 ` [PATCH v2 08/12] tracing: Use dyn_event framework for synthetic events Masami Hiramatsu
2018-11-05  9:04 ` [PATCH v2 09/12] tracing: Remove unneeded synth_event_mutex Masami Hiramatsu
2018-11-05  9:04 ` [PATCH v2 10/12] tracing: Remove orphaned trace_add/remove_event_call functions Masami Hiramatsu
2018-12-04 18:51   ` Steven Rostedt
2018-12-07  2:22     ` Masami Hiramatsu
2018-11-05  9:04 ` [PATCH v2 11/12] tracing: Add generic event-name based remove event method Masami Hiramatsu
2018-11-05  9:05 ` [PATCH v2 12/12] selftests/ftrace: Add testcases for dynamic event Masami Hiramatsu
2018-11-28  7:31 ` [PATCH v2 00/12] tracing: Unifying dynamic event interface Masami Hiramatsu
2018-11-28 23:42   ` Tom Zanussi [this message]
2018-11-29  3:46     ` Steven Rostedt
2018-11-29  5:20     ` Masami Hiramatsu
2018-11-29 15:08       ` Tom Zanussi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ff8ca3062cad3747a664e6888790cc3c58e24c7e.camel@linux.intel.com \
    --to=tom.zanussi@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=ravi.bangoria@linux.vnet.ibm.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git