On Wed, Jun 02, 2021 at 07:45:22AM +0800, Qu Wenruo wrote:
>
>
> On 2021/6/2 上午12:42, Darrick J. Wong wrote:
> > On Tue, Jun 01, 2021 at 03:31:33PM +0800, Qu Wenruo wrote:
> > > Sometimes developers want trace dump for certain test cases.
> > >
> > > Normally I just add "trace-cmd" calls in "check", but it would be much
> >
> > Heh, so do I!
> >
> > > better to let fstests to support ftrace dumping.
> > >
> > > This patchset will add basic ftrace dumping support by:
> > >
> > > - Clear all buffers before running each test
> > > - Start tracing before running each test
> > > - End tracing after test finished
> > > - Copy the trace to "$seqres.trace" if needed
> > > The condition is either:
> > > * $KEEP_TRACE environment is set to "yes"
> > > * The test case failed
> > >
> > > Currently we only support the main ftrace buffer, but all supporting
> > > functions have support for ftrace instances, for later expansion.
> >
> > I... did not know one could /have/ separate instances.
>
> So did I, until I checked the code of "trace-cmd clear".
>
> >
> > > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> > > ---
> > > check | 12 +++++++-
> > > common/ftrace | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > > common/rc | 1 +
> > > 3 files changed, 94 insertions(+), 1 deletion(-)
> > > create mode 100644 common/ftrace
> > >
> > > diff --git a/check b/check
> > > index ba192042..0a09dcf9 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -801,7 +801,7 @@ function run_section()
> > > fi
> > > # really going to try and run this one
> > > - rm -f $seqres.out.bad
> > > + rm -f $seqres.out.bad $seqres.trace
> > > # check if we really should run it
> > > _expunge_test $seqnum
> > > @@ -839,6 +839,10 @@ function run_section()
> > > # to be reported for each test
> > > (echo 1 > $DEBUGFS_MNT/clear_warn_once) > /dev/null 2>&1
> > > + # Clear previous trace and start new trace
> > > + _clear_trace_buffers
> > > + _start_trace
> >
> > How do you actually turn on specific tracepoints?
>
> Normally I use trace_printk(), thus no need to enable it manually.
>
> >
> > Or is the idea here to capture trace data for each test in a separate
> > file, and it's up to the ./check caller to set that up?
>
> My idea is, it should be the QA guy to set it up.
>
> Either using my stupid trace_printk() way, or enable those events before
> running the test.
>
> >
> > > +
> > > if [ "$DUMP_OUTPUT" = true ]; then
> > > _run_seq 2>&1 | tee $tmp.out
> > > # Because $? would get tee's return code
> > > @@ -848,6 +852,11 @@ function run_section()
> > > sts=$?
> > > fi
> > > + _end_trace
> > > + if [ "$KEEP_TRACE" == "yes" ]; then
> > > + _copy_trace "$seqres.trace"
> > > + fi
> > > +
> > > if [ -f core ]; then
> > > _dump_err_cont "[dumped core]"
> > > mv core $RESULT_BASE/$seqnum.core
> > > @@ -932,6 +941,7 @@ function run_section()
> > > # make sure we record the status of the last test we ran.
> > > if $err ; then
> > > + _copy_trace "$seqres.trace"
> > > bad="$bad $seqnum"
> > > n_bad=`expr $n_bad + 1`
> > > tc_status="fail"
> > > diff --git a/common/ftrace b/common/ftrace
> > > new file mode 100644
> > > index 00000000..36886484
> > > --- /dev/null
> > > +++ b/common/ftrace
> > > @@ -0,0 +1,82 @@
> > > +#
> > > +# Common ftrace related functions
> > > +#
> >
> > New file needs a SPDX header.
>
> Oh, forgot that.
>
> >
> > > +
> > > +TRACE_DIR="/sys/kernel/debug/tracing"
> > > +
> > > +_clear_trace_buffers()
> > > +{
> > > + if [ ! -d "${TRACE_DIR}" ]; then
> > > + return
> > > + fi
> > > +
> > > + # Clear the main buffer
> > > + echo 0 > "${TRACE_DIR}/trace"
> >
> > If one were already running trace-cmd record, will this mess up its
> > ability to collect trace data?
>
> I guess it may.
>
> But that shouldn't be a problem, all existing buffers are before running the
> tests, thus should be considered as noise.
Yes, that's true for the first test run, but this clears the trace
buffers between every test. If you start trace-cmd and run two tests,
isn't it possible that trace buffer content will be lost if trace-cmd
hasn't emptied the trace buffer when _clear_trace_buffers runs?
Alternate suggestion: Now that we know about the trace instance
functionality, what if the test runner passed a list of tracepoints to
./check? Then ./check can set up a new tracing instance for itself with
the specified tracepoints, and we can clear the buffer before each test
and arrange to dump the contents to the trace file while the test runs.
That will have the added benefit of not messing with any other tracing
tools that might be running.
--D
>
> >
> > > +
> > > + # Clear each instance buffer
> > > + for i in $(ls "${TRACE_DIR}/instances"); do
> > > + echo 0 > "${i}/trace"
> > > + done
> > > +}
> > > +
> > > +_start_trace()
> > > +{
> > > + instance=$1
> >
> > local instance="$1", please don't pollute the caller's environment.
>
> Oh, forgot it again...
>
> >
> > > +
> > > + if [ ! -d "${TRACE_DIR}" ]; then
> > > + return
> > > + fi
> > > +
> > > + if [ -z "${instance}" ]; then
> > > + echo 1 > "${TRACE_DIR}/tracing_on"
> > > + else
> > > + mkdir -p "${TRACE_DIR}/instances/${instance}"
> > > + echo 1 > "${TRACE_DIR}/instances/${instance}/tracing_on"
> > > + fi
> > > +}
> > > +
> > > +_end_trace()
> > > +{
> > > + instance=$1
> > > +
> > > + if [ ! -d "${TRACE_DIR}" ]; then
> > > + return
> > > + fi
> > > +
> > > + if [ -z "${instance}" ]; then
> > > + echo 0 > "${TRACE_DIR}/tracing_on"
> > > + else
> > > + mkdir -p "${TRACE_DIR}/instances/${instance}"
> >
> > Er, what's the logic here? Ensure that the instance exist so that we
> > can disable it?
>
> Oh, right, we should exit directly...
>
> This part is copies from _start_trace() obviously...
>
> Thanks,
> Qu
> >
> > --D
> >
> > > + echo 0 > "${TRACE_DIR}/instances/${instance}/tracing_on"
> > > + fi
> > > +}
> > > +
> > > +_remove_empty_trace()
> > > +{
> > > + file="$1"
> > > + if [ ! -f "$file" ]; then
> > > + return
> > > + fi
> > > +
> > > + if [ -z "$(head -n 15 $file | sed '/^#/d')" ]; then
> > > + rm $file
> > > + fi
> > > +}
> > > +
> > > +_copy_trace()
> > > +{
> > > + dest="$1"
> > > + instance="$2"
> > > +
> > > + if [ ! -d "${TRACE_DIR}" ]; then
> > > + return
> > > + fi
> > > +
> > > + if [ -z "${instance}" ]; then
> > > + cp "${TRACE_DIR}/trace" "$dest"
> > > + elif [ -d "${TRACE_DIR}/instances/${instance}" ]; then
> > > + cp "${TRACE_DIR}/instances/${instance}/trace" "$dest"
> > > + fi
> > > +
> > > + _remove_empty_trace "$dest"
> > > +}
> > > diff --git a/common/rc b/common/rc
> > > index 919028ef..f9de1517 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -3,6 +3,7 @@
> > > # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved.
> > > . common/config
> > > +. common/ftrace
> > > BC=$(which bc 2> /dev/null) || BC=
> > > --
> > > 2.31.1
> > >
> >
>
