Message-Id: <20181126230928.17883eecfaefd6a29cbbd1c3@kernel.org>
Date:   Mon, 26 Nov 2018 23:09:28 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Tom Zanussi <zanussi@...nel.org>
Cc:     rostedt@...dmis.org, tglx@...utronix.de, mhiramat@...nel.org,
        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
Subject: Re: [PATCH v7 00/16] tracing: Hist trigger snapshot and onchange
 additions

Hi Tom,

On Wed, 14 Nov 2018 14:17:57 -0600
Tom Zanussi <zanussi@...nel.org> wrote:

> From: Tom Zanussi <tom.zanussi@...ux.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?

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

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
> 


-- 
Masami Hiramatsu <mhiramat@...nel.org>

Powered by blists - more mailing lists