[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1531160735.20374.17.camel@kernel.org>
Date: Mon, 09 Jul 2018 13:25:35 -0500
From: Tom Zanussi <zanussi@...nel.org>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: rostedt@...dmis.org, tglx@...utronix.de, namhyung@...nel.org,
vedang.patel@...el.com, bigeasy@...utronix.de,
joel@...lfernandes.org, mathieu.desnoyers@...icios.com,
julia@...com, linux-kernel@...r.kernel.org,
linux-rt-users@...r.kernel.org,
Tom Zanussi <tom.zanussi@...ux.intel.com>
Subject: Re: [PATCH v2 0/7] tracing: Hist trigger snapshot and onchange
additions
Hi Masami,
On Sun, 2018-07-08 at 00:00 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On Mon, 2 Jul 2018 15:22:19 -0500
> Tom Zanussi <zanussi@...nel.org> wrote:
>
> > From: Tom Zanussi <tom.zanussi@...ux.intel.com>
> >
> > Hi,
> >
> > This is v2 of the hist trigger snapshot and onchange additions
> > patchset. It adds a couple fixes to problems flagged by the kbuild
> > test robot, but is otherwise the same as v1.
> >
> > Changes since v1:
> >
> > - added missing tracing_cond_snapshot_data() definition for when
> > CONFIG_TRACER_SNAPSHOT not defined
> > - removed an unnecessary WARN_ON() in track_data_snapshot_print()
> >
> >
> > Original text:
> >
> > This patchset adds some useful new functions to the hist
> > trigger code: a snapshot action and an onchange handler.
> >
> > In order to make it easier to add these and in the process make the
> > code more generic, I separated the code into explicit 'handlers'
> > and
> > 'actions', handlers being things like 'onmax' and 'onchange', and
> > 'actions' being things like 'take a snapshot' or 'save some
> > fields'.
>
> Sounds great!
>
> By the way, it seems that nowadays the syntax of trigger is
> very complicated. For example, we can set some 'actions' without
> handlers, but this introduce new 'handlers' on it.
>
> Could you consider not just extending it, but refactor it from
> the viewpoint of consistent and extensible syntax?
>
> e.g. if we support
>
> <actions> if <condition>
>
> syntax, why we can not do
>
> <actions> onchange(<var>)
It seems that doing this would restrict you to only one handler e.g.
you could no longer do something like:
...:onchange($var1).save(...):onmax($var2).snapshot()
I'm not sure how you would do that with your syntax.
On the other hand, if the most common use case is just a single handler
along with one or more actions, I think it would make sense to
provide a shorthand like you describe which just gets translated into
the longer more explicit form. Or were you thinking of something more
radical?
Thanks,
Tom
>
Powered by blists - more mailing lists