Re: [PATCH v7 00/16] tracing: Hist trigger snapshot and onchange additions

Tom Zanussi
2018-11-26T21:21:33Z

Hi Masami,

On Mon, 2018-11-26 at 23:09 +0900, Masami Hiramatsu wrote:
> Hi Tom,
>
> On Wed, 14 Nov 2018 14:17:57 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
> > From: Tom Zanussi <tom.zanussi@linux.intel.com>
> >
> > Hi,
> >
> > This is v7 of the hist trigger snapshot and onchange additions
> > patchset. It does a bit of refactoring to address the suggestions
> > made by Masami in v6.
>
> Thank you for fixing it.
>
> >
> > It also adds an additional patch to update the trigger/inter-event
> > testcases with SPDX license blurbs.
> >
> > BTW, I noticed that with the recent kselftest changes, I now get
> > mangled output when running the selftests, though I can still see
> > well
> > enough that the tests passed as expected. This happens with any of
> > the ftrace selftests and not just the trigger selftests. In my
> > case,
> > this is using the stock Terminal in Ubuntu 17.10, in case that
> > helps.
>
> Hmm, it should be fixed by
> 8096fbcf55c0 ("selftests/ftrace: Use colored output when available")
>
> Could you check your kernel has this commit?
>

Yes, it does have this commit.

> BTW, what terminal and environment (especially echo command)
> did you run your tests on? (It seems echo command didn't accept -e
> option)
>

For that system, I'm using Gnome terminal 3.24.2 and GNU bash, version
4.4.12(1)-release (x86_64-pc-linux-gnu) (Ubuntu 17.10).

If I change 'echo' to '/bin/echo' in e.g. prlog() it works fine, so it
must be the inbuilt bash echo that's not doing the right thing. I
thought it might be the xpg_echo option, but 'shopt -s xpg_echo'
doesn't have any effect.

I also tried on a Fedora 28 system (GNOME terminal 3.28.2, GNU bash,
version 4.4.23(1)-release (x86_64-redhat-linux-gnu), and it worked
fine.

Also, tried a Ubuntu 18.04.1 system (GNOME Terminal 3.28.1, GNU bash,
version 4.4.19(1)-release (x86_64-pc-linux-gnu) and in that case the
colors worked fine, but still got the '-e -n' and newlines in the
output:

-e -n [28] (instance) event trigger - test histogram modifiers
-e [PASS]
-e -n [29] (instance) event trigger - test histogram trigger
-e [PASS]

Again, substituting '/bin/echo' in prlog() fixed things in this case
too. I guess the builtin bash 'echo' can't be relied on..

Tom

> Thank you,
>
> > Example output:
> >
> > # ./ftracetest test.d/trigger/
> >
> > -e === Ftrace unit tests ===
> > -e -n [1] event trigger - test inter-event histogram trigger
> > expected fail actions
> > -e [\e[31mXFAIL\e[0m]
> > -e -n [2] event trigger - test extended error support
> > -e [\e[32mPASS\e[0m]
> > -e -n [3] event trigger - test field variable support
> > -e [\e[32mPASS\e[0m]
> > -e -n [4] event trigger - test inter-event combined histogram
> > trigger
> > -e [\e[32mPASS\e[0m]
> > -e -n [5] event trigger - test multiple actions on hist trigger
> > -e [\e[32mPASS\e[0m]
> > .
> > .
> > .
> > -e
> > -e # of passed: 31
> > -e # of failed: 0
> > -e # of unresolved: 0
> > -e # of untested: 0
> > -e # of unsupported: 0
> > -e # of xfailed: 1
> > -e # of undefined(test bug): 0
> >
> > v6->v7 changes:
> >
> > - Removed unnecessary HANDLER_ONMAX checks from
> > onmax_print()/create()
> > - Moved handler assignment to acion_parse()
> > - Changed goto in ATION_TRACE case in action_create() to return
> > - Changed EINVAL to EEXIST in action_create() ACTION_SAVE case
> > - Made the return logic in create_actions() more consistent
> > - Merged 'tracing: Move hist trigger key printing into a separate
> > function' into 'tracing: Add hist trigger snapshot() action'
> > - Updated the new snapshot, onchange, and trace test cases to
> > match
> > 4.20 kselftest changes.
> > - Added new xfail test case that make sure certain unsupported
> > handler/action combinations fail as expected.
> > - While updating the test cases, realized that the other
> > testcases
> > in the inter-event subdir needed SPDX license updates, so added
> > them.
> >
> > v5->v6 changes:
> >
> > - Added new Documentation patch explaining handler.action
> > - Added new README patch explaining handler.action
> > - Added separate snapshot() Documentation
> > - Added new snapshot() test case
> > - Updated README with snapshote()
> > - Added separate onchange() Documentation
> > - Added separate onchange() test case
> > - Updated README with onchange()
> > - Added separate trace() test case
> > - Updated README with trace() and <synthetic_event>() syntax
> >
> > v4->v5 changes:
> >
> > - added 'trace' keyword test case
> > - added 'onchange' handler test case
> >
> > v3->v4 changes:
> >
> > - added 'trace' keyword for generating synthetic events
> > - fix elt_data leak
> > - changed cond_update to cond_update_fn_t
> >
> > v2->v3 changes:
> >
> > - fixed problem where trace actions were only being allowed for
> > onmatch handlers - now trace actions can be used with any
> > handler.
> > - fixed problem where no action was being assigned to onmatch
> > handlers if save or snapshot actions were specified.
> >
> > v1->v2 changes:
> >
> > - 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'.
> >
> > The first few patches do that basic refactoring, which make it
> > easier
> > to add the subsequent changes that arbitrarily combine actions and
> > handlers.
> >
> > The fourth patch adds a 'conditional snapshot' capability that via
> > a
> > new tracing_snaphot_cond() function extends the existing snapshot
> > code. It allows the caller to associate some user data with the
> > snapshot that can be checked and saved in an update() callback
> > whose
> > return value determines whether the snapshot should be taken or
> > not.
> >
> > The remaining patches finally add the new snapshot action and
> > onchange
> > handler functionality - please see those patches for details and
> > some
> > examples.
> >
> > Thanks,
> >
> > Tom
> >
> >
> > The following changes since commit
> > ee474b81fe5aa5dc0faae920bf66240fbf55f891:
> >
> > tracing/kprobes: Fix strpbrk() argument order (2018-11-05
> > 09:47:14 -0500)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/zanussi/linux-
> > trace.git ftrace/hist-snapshot-onchange-v7
> >
> > Tom Zanussi (16):
> > tracing: Refactor hist trigger action code
> > tracing: Make hist trigger Documentation better reflect
> > actions/handlers
> > tracing: Add hist trigger handler.action documentation to README
> > tracing: Split up onmatch action data
> > tracing: Generalize hist trigger onmax and save action
> > tracing: Add conditional snapshot
> > tracing: Add hist trigger snapshot() action
> > tracing: Add hist trigger snapshot() action Documentation
> > tracing: Add hist trigger snapshot() action test case
> > tracing: Add hist trigger onchange() handler
> > tracing: Add hist trigger onchange() handler Documentation
> > tracing: Add hist trigger onchange() handler test case
> > tracing: Add alternative synthetic event trace action syntax
> > tracing: Add alternative synthetic event trace action test case
> > tracing: Add hist trigger action 'expected fail' test case
> > tracing: Add SPDX license GPL-2.0 license identifier to inter-
> > event
> > testcases
> >
> > Documentation/trace/histogram.rst | 285 +++++-
> > kernel/trace/trace.c | 177 +++-
> > kernel/trace/trace.h | 56 +-
> > kernel/trace/trace_events_hist.c | 1062
> > +++++++++++++++-----
> > kernel/trace/trace_events_trigger.c | 2 +-
> > kernel/trace/trace_sched_wakeup.c | 2 +-
> > .../inter-event/trigger-action-hist-xfail.tc | 30 +
> > .../inter-event/trigger-extended-error-support.tc | 1 +
> > .../inter-event/trigger-field-variable-support.tc | 1 +
> > .../trigger-inter-event-combined-hist.tc | 1 +
> > .../inter-event/trigger-multi-actions-accept.tc | 1 +
> > .../inter-event/trigger-onchange-action-hist.tc | 28 +
> > .../inter-event/trigger-onmatch-action-hist.tc | 1 +
> > .../trigger-onmatch-onmax-action-hist.tc | 1 +
> > .../inter-event/trigger-onmax-action-hist.tc | 1 +
> > .../inter-event/trigger-snapshot-action-hist.tc | 43 +
> > .../trigger-synthetic-event-createremove.tc | 1 +
> > .../inter-event/trigger-trace-action-hist.tc | 42 +
> > 18 files changed, 1433 insertions(+), 302 deletions(-)
> > create mode 100644
> > tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-
> > action-hist-xfail.tc
> > create mode 100644
> > tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-
> > onchange-action-hist.tc
> > create mode 100644
> > tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-
> > snapshot-action-hist.tc
> > create mode 100644
> > tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-
> > trace-action-hist.tc
> >
> > --
> > 2.14.1
> >
>
>