Re: [PATCH] fstests: add basic ftrace support

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





On 2021/6/4 上午12:26, Darrick J. Wong wrote:
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?

But in that case, we should have buffer saved for each test, no need for the manual trace-cmd to record.


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.

Yeah, that's the planned feature for the future.

But my current objective is just to remove the need for manual trace-cmd calls, so that with my trace_printk() added, I can test certain known reproducer without messing the fstests code.

So far it works well for my use case.

In the future, we may utilize certain instance, like 'fstests'.

Thanks,
Qu


--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









[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux