[PATCHSET 0/2] fstests: testing improvements

[PATCHSET 0/2] fstests: testing improvements

[Darrick J. Wong]

Hi all,

Here's a couple of patches that improve the test framework itself.  The
first patch adds a -smoketest option that will run the generic file IO
loop tests each for 4 minutes apiece.  The goal here is to provide a
quick means for developers to check that their changes didn't cause
major problems.

The second patch adds kernel gcov coverage reporting if the kernel is
set up to record such information.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=testing-improvements
---
 README              |    3 ++
 check               |   92 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 doc/group-names.txt |    1 +
 tests/generic/475   |    2 +
 tests/generic/476   |    2 +
 tests/generic/521   |    2 +
 tests/generic/522   |    2 +
 tests/generic/642   |    2 +
 8 files changed, 100 insertions(+), 6 deletions(-)

[PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a "-smoketest" parameter to check that will run generic
filesystem smoke testing for five minutes apiece.  Since there are only
five smoke tests, this is effectively a 16min super-quick test.

With gcov enabled, running these tests yields about ~75% coverage for
iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
btrfs.  Coverage was about ~65% for the pagecache.

Cc: tytso@mit.edu
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 check               |    6 +++++-
 doc/group-names.txt |    1 +
 tests/generic/475   |    2 +-
 tests/generic/476   |    2 +-
 tests/generic/521   |    2 +-
 tests/generic/522   |    2 +-
 tests/generic/642   |    2 +-
 7 files changed, 11 insertions(+), 6 deletions(-)


diff --git a/check b/check
index 89e7e7bf20..97c7c4c7d1 100755
--- a/check
+++ b/check
@@ -68,6 +68,7 @@ check options
     -pvfs2		test PVFS2
     -tmpfs		test TMPFS
     -ubifs		test ubifs
+    -smoketest		run smoke tests for 4min each
     -l			line mode diff
     -udiff		show unified diff (default)
     -n			show me, do not run tests
@@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
 		FSTYP=overlay
 		export OVERLAY=true
 		;;
-
+	-smoketest)
+		SOAK_DURATION="4m"
+		GROUP_LIST="smoketest"
+		;;
 	-g)	group=$2 ; shift ;
 		GROUP_LIST="$GROUP_LIST ${group//,/ }"
 		;;
diff --git a/doc/group-names.txt b/doc/group-names.txt
index 1c35a39432..c3dcca3755 100644
--- a/doc/group-names.txt
+++ b/doc/group-names.txt
@@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
 send			btrfs send/receive
 shrinkfs		decreasing the size of a filesystem
 shutdown		FS_IOC_SHUTDOWN ioctl
+smoketest		Simple smoke tests
 snapshot		btrfs snapshots
 soak			long running soak tests whose runtime can be controlled
                         directly by setting the SOAK_DURATION variable
diff --git a/tests/generic/475 b/tests/generic/475
index 0cbf5131c2..ce7fe013b1 100755
--- a/tests/generic/475
+++ b/tests/generic/475
@@ -12,7 +12,7 @@
 # testing efforts.
 #
 . ./common/preamble
-_begin_fstest shutdown auto log metadata eio recoveryloop
+_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
 
 # Override the default cleanup function.
 _cleanup()
diff --git a/tests/generic/476 b/tests/generic/476
index 8e93b73457..b1ae4df4d4 100755
--- a/tests/generic/476
+++ b/tests/generic/476
@@ -8,7 +8,7 @@
 # bugs in the write path.
 #
 . ./common/preamble
-_begin_fstest auto rw long_rw stress soak
+_begin_fstest auto rw long_rw stress soak smoketest
 
 # Override the default cleanup function.
 _cleanup()
diff --git a/tests/generic/521 b/tests/generic/521
index 22dd31a8ec..0956e50171 100755
--- a/tests/generic/521
+++ b/tests/generic/521
@@ -7,7 +7,7 @@
 # Long-soak directio fsx test
 #
 . ./common/preamble
-_begin_fstest soak long_rw
+_begin_fstest soak long_rw smoketest
 
 # Import common functions.
 . ./common/filter
diff --git a/tests/generic/522 b/tests/generic/522
index f0cbcb245c..0e4e6009ed 100755
--- a/tests/generic/522
+++ b/tests/generic/522
@@ -7,7 +7,7 @@
 # Long-soak buffered fsx test
 #
 . ./common/preamble
-_begin_fstest soak long_rw
+_begin_fstest soak long_rw smoketest
 
 # Import common functions.
 . ./common/filter
diff --git a/tests/generic/642 b/tests/generic/642
index eba90903a3..e6a475a8b5 100755
--- a/tests/generic/642
+++ b/tests/generic/642
@@ -8,7 +8,7 @@
 # bugs in the xattr code.
 #
 . ./common/preamble
-_begin_fstest auto soak attr long_rw stress
+_begin_fstest auto soak attr long_rw stress smoketest
 
 _cleanup()
 {

[PATCH 2/2] check: generate gcov code coverage reports at the end of each section

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Support collecting kernel code coverage information as reported in
debugfs.  At the start of each section, we reset the gcov counters;
during the section wrapup, we'll collect the kernel gcov data.

If lcov is installed and the kernel source code is available, it will
also generate a nice html report.  If a CLI web browser is available, it
will also format the html report into text for easy grepping.

This requires the test runner to set REPORT_GCOV=1 explicitly and gcov
to be enabled in the kernel.

Cc: tytso@mit.edu
Cc: kent.overstreet@linux.dev
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 README |    3 ++
 check  |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)


diff --git a/README b/README
index 9790334db1..ccfdcbe703 100644
--- a/README
+++ b/README
@@ -249,6 +249,9 @@ Kernel/Modules related configuration:
    to "forever" and we'll wait forever until the module is gone.
  - Set KCONFIG_PATH to specify your preferred location of kernel config
    file. The config is used by tests to check if kernel feature is enabled.
+ - Set REPORT_GCOV to a directory path to make lcov and genhtml generate
+   html reports from any gcov code coverage data collected by the kernel.
+   If REPORT_GCOV is set to 1, the report will be written to $REPORT_DIR/gcov/.
 
 Test control:
  - Set LOAD_FACTOR to a nonzero positive integer to increase the amount of
diff --git a/check b/check
index 97c7c4c7d1..3e6f27c653 100755
--- a/check
+++ b/check
@@ -451,6 +451,87 @@ _global_log() {
 	fi
 }
 
+GCOV_DIR=/sys/kernel/debug/gcov
+
+# Find the topmost directories of the .gcno directory hierarchy
+_gcov_find_topdirs() {
+	find "${GCOV_DIR}/" -name '*.gcno' -printf '%d|%h\n' | \
+		sort -g -k 1 | \
+		uniq | \
+		$AWK_PROG -F '|' 'BEGIN { x = -1 } { if (x < 0) x = $1; if ($1 == x) printf("%s\n", $2);}'
+}
+
+# Generate lcov html report from kernel gcov data if configured
+_gcov_generate_report() {
+	unset REPORT_GCOV	# don't trigger multiple times if ^C
+
+	local output_dir="$1"
+	test -n "${output_dir}" || return
+	test "$output_dir" = "1" && output_dir="$REPORT_DIR/gcov"
+
+	# Kernel support built in?
+	test -d "$GCOV_DIR" || return
+
+	readarray -t gcno_dirs < <(_gcov_find_topdirs)
+	test "${#gcno_dirs[@]}" -gt 0 || return
+
+	mkdir -p "${output_dir}/"
+
+	# Collect raw coverage data from the kernel
+	readarray -t source_dirs < <(find "${GCOV_DIR}/" -mindepth 1 -maxdepth 1 -type d)
+	for dir in "${source_dirs[@]}"; do
+		cp -p -R -d -u "${dir}" "${output_dir}/"
+	done
+
+	# If lcov is installed, use it to summarize the gcda data.
+	# If it is not installed, there's no point in going forward
+	command -v lcov > /dev/null || return
+	local lcov=(lcov --exclude 'include*' --capture)
+	lcov+=(--output-file "${output_dir}/gcov.report")
+	for d in "${gcno_dirs[@]}"; do
+		lcov+=(--directory "${d}")
+	done
+
+	# Generate a detailed HTML report from the summary
+	local gcov_start_time="$(date --date="${fstests_start_time:-now}")"
+	local genhtml=()
+	if command -v genhtml > /dev/null; then
+		genhtml+=(genhtml -o "${output_dir}/" "${output_dir}/gcov.report")
+		genhtml+=(--title "fstests on $(hostname -s) @ ${gcov_start_time}" --legend)
+	fi
+
+	# Try to convert the HTML report summary as text for easier grepping if
+	# there's an HTML renderer present
+	local totext=()
+	test "${#totext[@]}" -eq 0 && \
+		command -v lynx &>/dev/null && \
+		totext=(lynx -dump "${output_dir}/index.html" -width 120 -nonumbers -nolist)
+	test "${#totext[@]}" -eq 0 && \
+		command -v links &>/dev/null && \
+		totext=(links -dump "${output_dir}/index.html" -width 120)
+	test "${#totext[@]}" -eq 0 && \
+		command -v elinks &>/dev/null && \
+		totext=(elinks -dump "${output_dir}/index.html" --dump-width 120 --no-numbering --no-references)
+
+	# Analyze kernel data
+	"${lcov[@]}" > "${output_dir}/gcov.stdout" 2> "${output_dir}/gcov.stderr"
+	test "${#genhtml[@]}" -ne 0 && \
+		"${genhtml[@]}" >> "${output_dir}/gcov.stdout" 2>> "${output_dir}/gcov.stderr"
+	test "${#totext[@]}" -ne 0 && \
+		"${totext[@]}" > "${output_dir}/index.txt" 2>> "${output_dir}/gcov.stderr"
+}
+
+# Reset gcov usage data
+_gcov_reset() {
+	test -n "${REPORT_GCOV}" || return
+
+	if [ -w "${GCOV_DIR}/reset" ]; then
+		echo 1 > "${GCOV_DIR}/reset"
+	else
+		unset REPORT_GCOV
+	fi
+}
+
 _wrapup()
 {
 	seq="check"
@@ -527,6 +608,10 @@ _wrapup()
 					     "${#bad[*]}" "${#notrun[*]}" \
 					     "$((sect_stop - sect_start))"
 		fi
+
+		# Generate code coverage report
+		test -n "$REPORT_GCOV" && _gcov_generate_report "$REPORT_GCOV"
+
 		needwrap=false
 	fi
 
@@ -801,6 +886,7 @@ function run_section()
 	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
 	fi
 	echo
+	_gcov_reset
 	needwrap=true
 
 	if [ ! -z "$SCRATCH_DEV" ]; then

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a "-smoketest" parameter to check that will run generic
> filesystem smoke testing for five minutes apiece.  Since there are only
> five smoke tests, this is effectively a 16min super-quick test.
> 
> With gcov enabled, running these tests yields about ~75% coverage for
> iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> btrfs.  Coverage was about ~65% for the pagecache.
> 
> Cc: tytso@mit.edu
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  check               |    6 +++++-
>  doc/group-names.txt |    1 +
>  tests/generic/475   |    2 +-
>  tests/generic/476   |    2 +-
>  tests/generic/521   |    2 +-
>  tests/generic/522   |    2 +-
>  tests/generic/642   |    2 +-
>  7 files changed, 11 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/check b/check
> index 89e7e7bf20..97c7c4c7d1 100755
> --- a/check
> +++ b/check
> @@ -68,6 +68,7 @@ check options
>      -pvfs2		test PVFS2
>      -tmpfs		test TMPFS
>      -ubifs		test ubifs
> +    -smoketest		run smoke tests for 4min each

We have both "smoketest" and "smoke", that's a bit confused :)

>      -l			line mode diff
>      -udiff		show unified diff (default)
>      -n			show me, do not run tests
> @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
>  		FSTYP=overlay
>  		export OVERLAY=true
>  		;;
> -
> +	-smoketest)

Hmm... I'm wondering if it's worth having a specific running option for
someone test group. If each "meaningful" testing way need a specific check
option, the ./check file will be too complicated.

If we need some recommended test ways, how about make some separated wrappers
of ./check? For example:

# mkdir fstests/runtest/
# cat > fstests/runtest/smoketest <<EOF
export SOAK_DURATION="4m"
./check -g smoketest
EOF

Of course you can write more codes in it.

Thanks,
Zorro

> +		SOAK_DURATION="4m"
> +		GROUP_LIST="smoketest"
> +		;;
>  	-g)	group=$2 ; shift ;
>  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
>  		;;
> diff --git a/doc/group-names.txt b/doc/group-names.txt
> index 1c35a39432..c3dcca3755 100644
> --- a/doc/group-names.txt
> +++ b/doc/group-names.txt
> @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
>  send			btrfs send/receive
>  shrinkfs		decreasing the size of a filesystem
>  shutdown		FS_IOC_SHUTDOWN ioctl
> +smoketest		Simple smoke tests
>  snapshot		btrfs snapshots
>  soak			long running soak tests whose runtime can be controlled
>                          directly by setting the SOAK_DURATION variable
> diff --git a/tests/generic/475 b/tests/generic/475
> index 0cbf5131c2..ce7fe013b1 100755
> --- a/tests/generic/475
> +++ b/tests/generic/475
> @@ -12,7 +12,7 @@
>  # testing efforts.
>  #
>  . ./common/preamble
> -_begin_fstest shutdown auto log metadata eio recoveryloop
> +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
>  
>  # Override the default cleanup function.
>  _cleanup()
> diff --git a/tests/generic/476 b/tests/generic/476
> index 8e93b73457..b1ae4df4d4 100755
> --- a/tests/generic/476
> +++ b/tests/generic/476
> @@ -8,7 +8,7 @@
>  # bugs in the write path.
>  #
>  . ./common/preamble
> -_begin_fstest auto rw long_rw stress soak
> +_begin_fstest auto rw long_rw stress soak smoketest
>  
>  # Override the default cleanup function.
>  _cleanup()
> diff --git a/tests/generic/521 b/tests/generic/521
> index 22dd31a8ec..0956e50171 100755
> --- a/tests/generic/521
> +++ b/tests/generic/521
> @@ -7,7 +7,7 @@
>  # Long-soak directio fsx test
>  #
>  . ./common/preamble
> -_begin_fstest soak long_rw
> +_begin_fstest soak long_rw smoketest
>  
>  # Import common functions.
>  . ./common/filter
> diff --git a/tests/generic/522 b/tests/generic/522
> index f0cbcb245c..0e4e6009ed 100755
> --- a/tests/generic/522
> +++ b/tests/generic/522
> @@ -7,7 +7,7 @@
>  # Long-soak buffered fsx test
>  #
>  . ./common/preamble
> -_begin_fstest soak long_rw
> +_begin_fstest soak long_rw smoketest
>  
>  # Import common functions.
>  . ./common/filter
> diff --git a/tests/generic/642 b/tests/generic/642
> index eba90903a3..e6a475a8b5 100755
> --- a/tests/generic/642
> +++ b/tests/generic/642
> @@ -8,7 +8,7 @@
>  # bugs in the xattr code.
>  #
>  . ./common/preamble
> -_begin_fstest auto soak attr long_rw stress
> +_begin_fstest auto soak attr long_rw stress smoketest
>  
>  _cleanup()
>  {
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

On Wed, Jul 19, 2023 at 11:10:24PM +0800, Zorro Lang wrote:
> On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a "-smoketest" parameter to check that will run generic
> > filesystem smoke testing for five minutes apiece.  Since there are only
> > five smoke tests, this is effectively a 16min super-quick test.
> > 
> > With gcov enabled, running these tests yields about ~75% coverage for
> > iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> > btrfs.  Coverage was about ~65% for the pagecache.
> > 
> > Cc: tytso@mit.edu
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  check               |    6 +++++-
> >  doc/group-names.txt |    1 +
> >  tests/generic/475   |    2 +-
> >  tests/generic/476   |    2 +-
> >  tests/generic/521   |    2 +-
> >  tests/generic/522   |    2 +-
> >  tests/generic/642   |    2 +-
> >  7 files changed, 11 insertions(+), 6 deletions(-)
> > 
> > 
> > diff --git a/check b/check
> > index 89e7e7bf20..97c7c4c7d1 100755
> > --- a/check
> > +++ b/check
> > @@ -68,6 +68,7 @@ check options
> >      -pvfs2		test PVFS2
> >      -tmpfs		test TMPFS
> >      -ubifs		test ubifs
> > +    -smoketest		run smoke tests for 4min each
> 
> We have both "smoketest" and "smoke", that's a bit confused :)

We do?  git grep doesn't show anything other than what I added:

$ git grep smoke
check:71:    -smoketest         run smoke tests for 4min each
check:294:      -smoketest)
check:296:              GROUP_LIST="smoketest"
doc/group-names.txt:123:smoketest               Simple smoke tests
tests/generic/475:15:_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
tests/generic/476:11:_begin_fstest auto rw long_rw stress soak smoketest
tests/generic/521:10:_begin_fstest soak long_rw smoketest
tests/generic/522:10:_begin_fstest soak long_rw smoketest
tests/generic/533:9:# Simple attr smoke tests for user EAs, dereived from generic/097.
tests/generic/642:11:_begin_fstest auto soak attr long_rw stress smoketest

> >      -l			line mode diff
> >      -udiff		show unified diff (default)
> >      -n			show me, do not run tests
> > @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
> >  		FSTYP=overlay
> >  		export OVERLAY=true
> >  		;;
> > -
> > +	-smoketest)
> 
> Hmm... I'm wondering if it's worth having a specific running option for
> someone test group. If each "meaningful" testing way need a specific check
> option, the ./check file will be too complicated.
> 
> If we need some recommended test ways, how about make some separated wrappers
> of ./check? For example:
> 
> # mkdir fstests/runtest/
> # cat > fstests/runtest/smoketest <<EOF
> export SOAK_DURATION="4m"
> ./check -g smoketest
> EOF
> 
> Of course you can write more codes in it.

The goal here was to give casual developers an easy way to run a quick
15 minute exercise *without* having to write wrapper scripts or type
all that in every time.  Compare:

$ ./check -smoketest

vs.

$ SOAK_DURATION=4m ./check -g smoketest

--D

> Thanks,
> Zorro
> 
> > +		SOAK_DURATION="4m"
> > +		GROUP_LIST="smoketest"
> > +		;;
> >  	-g)	group=$2 ; shift ;
> >  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> >  		;;
> > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > index 1c35a39432..c3dcca3755 100644
> > --- a/doc/group-names.txt
> > +++ b/doc/group-names.txt
> > @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
> >  send			btrfs send/receive
> >  shrinkfs		decreasing the size of a filesystem
> >  shutdown		FS_IOC_SHUTDOWN ioctl
> > +smoketest		Simple smoke tests
> >  snapshot		btrfs snapshots
> >  soak			long running soak tests whose runtime can be controlled
> >                          directly by setting the SOAK_DURATION variable
> > diff --git a/tests/generic/475 b/tests/generic/475
> > index 0cbf5131c2..ce7fe013b1 100755
> > --- a/tests/generic/475
> > +++ b/tests/generic/475
> > @@ -12,7 +12,7 @@
> >  # testing efforts.
> >  #
> >  . ./common/preamble
> > -_begin_fstest shutdown auto log metadata eio recoveryloop
> > +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> >  
> >  # Override the default cleanup function.
> >  _cleanup()
> > diff --git a/tests/generic/476 b/tests/generic/476
> > index 8e93b73457..b1ae4df4d4 100755
> > --- a/tests/generic/476
> > +++ b/tests/generic/476
> > @@ -8,7 +8,7 @@
> >  # bugs in the write path.
> >  #
> >  . ./common/preamble
> > -_begin_fstest auto rw long_rw stress soak
> > +_begin_fstest auto rw long_rw stress soak smoketest
> >  
> >  # Override the default cleanup function.
> >  _cleanup()
> > diff --git a/tests/generic/521 b/tests/generic/521
> > index 22dd31a8ec..0956e50171 100755
> > --- a/tests/generic/521
> > +++ b/tests/generic/521
> > @@ -7,7 +7,7 @@
> >  # Long-soak directio fsx test
> >  #
> >  . ./common/preamble
> > -_begin_fstest soak long_rw
> > +_begin_fstest soak long_rw smoketest
> >  
> >  # Import common functions.
> >  . ./common/filter
> > diff --git a/tests/generic/522 b/tests/generic/522
> > index f0cbcb245c..0e4e6009ed 100755
> > --- a/tests/generic/522
> > +++ b/tests/generic/522
> > @@ -7,7 +7,7 @@
> >  # Long-soak buffered fsx test
> >  #
> >  . ./common/preamble
> > -_begin_fstest soak long_rw
> > +_begin_fstest soak long_rw smoketest
> >  
> >  # Import common functions.
> >  . ./common/filter
> > diff --git a/tests/generic/642 b/tests/generic/642
> > index eba90903a3..e6a475a8b5 100755
> > --- a/tests/generic/642
> > +++ b/tests/generic/642
> > @@ -8,7 +8,7 @@
> >  # bugs in the xattr code.
> >  #
> >  . ./common/preamble
> > -_begin_fstest auto soak attr long_rw stress
> > +_begin_fstest auto soak attr long_rw stress smoketest
> >  
> >  _cleanup()
> >  {
> > 
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Wed, Jul 19, 2023 at 08:29:07AM -0700, Darrick J. Wong wrote:
> On Wed, Jul 19, 2023 at 11:10:24PM +0800, Zorro Lang wrote:
> > On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Create a "-smoketest" parameter to check that will run generic
> > > filesystem smoke testing for five minutes apiece.  Since there are only
> > > five smoke tests, this is effectively a 16min super-quick test.
> > > 
> > > With gcov enabled, running these tests yields about ~75% coverage for
> > > iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> > > btrfs.  Coverage was about ~65% for the pagecache.
> > > 
> > > Cc: tytso@mit.edu
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  check               |    6 +++++-
> > >  doc/group-names.txt |    1 +
> > >  tests/generic/475   |    2 +-
> > >  tests/generic/476   |    2 +-
> > >  tests/generic/521   |    2 +-
> > >  tests/generic/522   |    2 +-
> > >  tests/generic/642   |    2 +-
> > >  7 files changed, 11 insertions(+), 6 deletions(-)
> > > 
> > > 
> > > diff --git a/check b/check
> > > index 89e7e7bf20..97c7c4c7d1 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -68,6 +68,7 @@ check options
> > >      -pvfs2		test PVFS2
> > >      -tmpfs		test TMPFS
> > >      -ubifs		test ubifs
> > > +    -smoketest		run smoke tests for 4min each
> > 
> > We have both "smoketest" and "smoke", that's a bit confused :)
> 
> We do?  git grep doesn't show anything other than what I added:
> 
> $ git grep smoke
> check:71:    -smoketest         run smoke tests for 4min each
> check:294:      -smoketest)
> check:296:              GROUP_LIST="smoketest"
> doc/group-names.txt:123:smoketest               Simple smoke tests
> tests/generic/475:15:_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> tests/generic/476:11:_begin_fstest auto rw long_rw stress soak smoketest
> tests/generic/521:10:_begin_fstest soak long_rw smoketest
> tests/generic/522:10:_begin_fstest soak long_rw smoketest
> tests/generic/533:9:# Simple attr smoke tests for user EAs, dereived from generic/097.
> tests/generic/642:11:_begin_fstest auto soak attr long_rw stress smoketest

Oh, sorry, my memory is a bit of jumbled ...

> 
> > >      -l			line mode diff
> > >      -udiff		show unified diff (default)
> > >      -n			show me, do not run tests
> > > @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
> > >  		FSTYP=overlay
> > >  		export OVERLAY=true
> > >  		;;
> > > -
> > > +	-smoketest)
> > 
> > Hmm... I'm wondering if it's worth having a specific running option for
> > someone test group. If each "meaningful" testing way need a specific check
> > option, the ./check file will be too complicated.
> > 
> > If we need some recommended test ways, how about make some separated wrappers
> > of ./check? For example:
> > 
> > # mkdir fstests/runtest/
> > # cat > fstests/runtest/smoketest <<EOF
> > export SOAK_DURATION="4m"
> > ./check -g smoketest
> > EOF
> > 
> > Of course you can write more codes in it.
> 
> The goal here was to give casual developers an easy way to run a quick
> 15 minute exercise *without* having to write wrapper scripts or type
> all that in every time.  Compare:
> 
> $ ./check -smoketest
> 
> vs.
> 
> $ SOAK_DURATION=4m ./check -g smoketest

Oh, I don't mean let users write that wrapper, I mean we provide some wrapper
scripts (to be recommended). E.g.

# ./runtest/smoaktest

If we give "smoaktest" a specific run option, what will we do if more people
want to add more options like that?

But a wrapper is not an offical running option, it's just a reference which
can be used directly or can be copied. Then we can have more wrappers from
each fs expert as reference, to recommend other users how to use fstests
specially. And I don't need to add options for each of them. What do you think?

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > > +		SOAK_DURATION="4m"
> > > +		GROUP_LIST="smoketest"
> > > +		;;
> > >  	-g)	group=$2 ; shift ;
> > >  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> > >  		;;
> > > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > > index 1c35a39432..c3dcca3755 100644
> > > --- a/doc/group-names.txt
> > > +++ b/doc/group-names.txt
> > > @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
> > >  send			btrfs send/receive
> > >  shrinkfs		decreasing the size of a filesystem
> > >  shutdown		FS_IOC_SHUTDOWN ioctl
> > > +smoketest		Simple smoke tests
> > >  snapshot		btrfs snapshots
> > >  soak			long running soak tests whose runtime can be controlled
> > >                          directly by setting the SOAK_DURATION variable
> > > diff --git a/tests/generic/475 b/tests/generic/475
> > > index 0cbf5131c2..ce7fe013b1 100755
> > > --- a/tests/generic/475
> > > +++ b/tests/generic/475
> > > @@ -12,7 +12,7 @@
> > >  # testing efforts.
> > >  #
> > >  . ./common/preamble
> > > -_begin_fstest shutdown auto log metadata eio recoveryloop
> > > +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > >  
> > >  # Override the default cleanup function.
> > >  _cleanup()
> > > diff --git a/tests/generic/476 b/tests/generic/476
> > > index 8e93b73457..b1ae4df4d4 100755
> > > --- a/tests/generic/476
> > > +++ b/tests/generic/476
> > > @@ -8,7 +8,7 @@
> > >  # bugs in the write path.
> > >  #
> > >  . ./common/preamble
> > > -_begin_fstest auto rw long_rw stress soak
> > > +_begin_fstest auto rw long_rw stress soak smoketest
> > >  
> > >  # Override the default cleanup function.
> > >  _cleanup()
> > > diff --git a/tests/generic/521 b/tests/generic/521
> > > index 22dd31a8ec..0956e50171 100755
> > > --- a/tests/generic/521
> > > +++ b/tests/generic/521
> > > @@ -7,7 +7,7 @@
> > >  # Long-soak directio fsx test
> > >  #
> > >  . ./common/preamble
> > > -_begin_fstest soak long_rw
> > > +_begin_fstest soak long_rw smoketest
> > >  
> > >  # Import common functions.
> > >  . ./common/filter
> > > diff --git a/tests/generic/522 b/tests/generic/522
> > > index f0cbcb245c..0e4e6009ed 100755
> > > --- a/tests/generic/522
> > > +++ b/tests/generic/522
> > > @@ -7,7 +7,7 @@
> > >  # Long-soak buffered fsx test
> > >  #
> > >  . ./common/preamble
> > > -_begin_fstest soak long_rw
> > > +_begin_fstest soak long_rw smoketest
> > >  
> > >  # Import common functions.
> > >  . ./common/filter
> > > diff --git a/tests/generic/642 b/tests/generic/642
> > > index eba90903a3..e6a475a8b5 100755
> > > --- a/tests/generic/642
> > > +++ b/tests/generic/642
> > > @@ -8,7 +8,7 @@
> > >  # bugs in the xattr code.
> > >  #
> > >  . ./common/preamble
> > > -_begin_fstest auto soak attr long_rw stress
> > > +_begin_fstest auto soak attr long_rw stress smoketest
> > >  
> > >  _cleanup()
> > >  {
> > > 
> > 
> 

Re: [PATCH 2/2] check: generate gcov code coverage reports at the end of each section

[Zorro Lang]

On Tue, Jul 18, 2023 at 06:11:01PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Support collecting kernel code coverage information as reported in
> debugfs.  At the start of each section, we reset the gcov counters;
> during the section wrapup, we'll collect the kernel gcov data.
> 
> If lcov is installed and the kernel source code is available, it will
> also generate a nice html report.  If a CLI web browser is available, it
> will also format the html report into text for easy grepping.
> 
> This requires the test runner to set REPORT_GCOV=1 explicitly and gcov
> to be enabled in the kernel.
> 
> Cc: tytso@mit.edu
> Cc: kent.overstreet@linux.dev
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Hi Darrick,

Is that possible to split this function from check script to tools/ ?

Thanks,
Zorro

>  README |    3 ++
>  check  |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
> 
> 
> diff --git a/README b/README
> index 9790334db1..ccfdcbe703 100644
> --- a/README
> +++ b/README
> @@ -249,6 +249,9 @@ Kernel/Modules related configuration:
>     to "forever" and we'll wait forever until the module is gone.
>   - Set KCONFIG_PATH to specify your preferred location of kernel config
>     file. The config is used by tests to check if kernel feature is enabled.
> + - Set REPORT_GCOV to a directory path to make lcov and genhtml generate
> +   html reports from any gcov code coverage data collected by the kernel.
> +   If REPORT_GCOV is set to 1, the report will be written to $REPORT_DIR/gcov/.
>  
>  Test control:
>   - Set LOAD_FACTOR to a nonzero positive integer to increase the amount of
> diff --git a/check b/check
> index 97c7c4c7d1..3e6f27c653 100755
> --- a/check
> +++ b/check
> @@ -451,6 +451,87 @@ _global_log() {
>  	fi
>  }
>  
> +GCOV_DIR=/sys/kernel/debug/gcov
> +
> +# Find the topmost directories of the .gcno directory hierarchy
> +_gcov_find_topdirs() {
> +	find "${GCOV_DIR}/" -name '*.gcno' -printf '%d|%h\n' | \
> +		sort -g -k 1 | \
> +		uniq | \
> +		$AWK_PROG -F '|' 'BEGIN { x = -1 } { if (x < 0) x = $1; if ($1 == x) printf("%s\n", $2);}'
> +}
> +
> +# Generate lcov html report from kernel gcov data if configured
> +_gcov_generate_report() {
> +	unset REPORT_GCOV	# don't trigger multiple times if ^C
> +
> +	local output_dir="$1"
> +	test -n "${output_dir}" || return
> +	test "$output_dir" = "1" && output_dir="$REPORT_DIR/gcov"
> +
> +	# Kernel support built in?
> +	test -d "$GCOV_DIR" || return
> +
> +	readarray -t gcno_dirs < <(_gcov_find_topdirs)
> +	test "${#gcno_dirs[@]}" -gt 0 || return
> +
> +	mkdir -p "${output_dir}/"
> +
> +	# Collect raw coverage data from the kernel
> +	readarray -t source_dirs < <(find "${GCOV_DIR}/" -mindepth 1 -maxdepth 1 -type d)
> +	for dir in "${source_dirs[@]}"; do
> +		cp -p -R -d -u "${dir}" "${output_dir}/"
> +	done
> +
> +	# If lcov is installed, use it to summarize the gcda data.
> +	# If it is not installed, there's no point in going forward
> +	command -v lcov > /dev/null || return
> +	local lcov=(lcov --exclude 'include*' --capture)
> +	lcov+=(--output-file "${output_dir}/gcov.report")
> +	for d in "${gcno_dirs[@]}"; do
> +		lcov+=(--directory "${d}")
> +	done
> +
> +	# Generate a detailed HTML report from the summary
> +	local gcov_start_time="$(date --date="${fstests_start_time:-now}")"
> +	local genhtml=()
> +	if command -v genhtml > /dev/null; then
> +		genhtml+=(genhtml -o "${output_dir}/" "${output_dir}/gcov.report")
> +		genhtml+=(--title "fstests on $(hostname -s) @ ${gcov_start_time}" --legend)
> +	fi
> +
> +	# Try to convert the HTML report summary as text for easier grepping if
> +	# there's an HTML renderer present
> +	local totext=()
> +	test "${#totext[@]}" -eq 0 && \
> +		command -v lynx &>/dev/null && \
> +		totext=(lynx -dump "${output_dir}/index.html" -width 120 -nonumbers -nolist)
> +	test "${#totext[@]}" -eq 0 && \
> +		command -v links &>/dev/null && \
> +		totext=(links -dump "${output_dir}/index.html" -width 120)
> +	test "${#totext[@]}" -eq 0 && \
> +		command -v elinks &>/dev/null && \
> +		totext=(elinks -dump "${output_dir}/index.html" --dump-width 120 --no-numbering --no-references)
> +
> +	# Analyze kernel data
> +	"${lcov[@]}" > "${output_dir}/gcov.stdout" 2> "${output_dir}/gcov.stderr"
> +	test "${#genhtml[@]}" -ne 0 && \
> +		"${genhtml[@]}" >> "${output_dir}/gcov.stdout" 2>> "${output_dir}/gcov.stderr"
> +	test "${#totext[@]}" -ne 0 && \
> +		"${totext[@]}" > "${output_dir}/index.txt" 2>> "${output_dir}/gcov.stderr"
> +}
> +
> +# Reset gcov usage data
> +_gcov_reset() {
> +	test -n "${REPORT_GCOV}" || return
> +
> +	if [ -w "${GCOV_DIR}/reset" ]; then
> +		echo 1 > "${GCOV_DIR}/reset"
> +	else
> +		unset REPORT_GCOV
> +	fi
> +}
> +
>  _wrapup()
>  {
>  	seq="check"
> @@ -527,6 +608,10 @@ _wrapup()
>  					     "${#bad[*]}" "${#notrun[*]}" \
>  					     "$((sect_stop - sect_start))"
>  		fi
> +
> +		# Generate code coverage report
> +		test -n "$REPORT_GCOV" && _gcov_generate_report "$REPORT_GCOV"
> +
>  		needwrap=false
>  	fi
>  
> @@ -801,6 +886,7 @@ function run_section()
>  	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
>  	fi
>  	echo
> +	_gcov_reset
>  	needwrap=true
>  
>  	if [ ! -z "$SCRATCH_DEV" ]; then
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

On Thu, Jul 20, 2023 at 12:11:15AM +0800, Zorro Lang wrote:
> On Wed, Jul 19, 2023 at 08:29:07AM -0700, Darrick J. Wong wrote:
> > On Wed, Jul 19, 2023 at 11:10:24PM +0800, Zorro Lang wrote:
> > > On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Create a "-smoketest" parameter to check that will run generic
> > > > filesystem smoke testing for five minutes apiece.  Since there are only
> > > > five smoke tests, this is effectively a 16min super-quick test.
> > > > 
> > > > With gcov enabled, running these tests yields about ~75% coverage for
> > > > iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> > > > btrfs.  Coverage was about ~65% for the pagecache.
> > > > 
> > > > Cc: tytso@mit.edu
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > >  check               |    6 +++++-
> > > >  doc/group-names.txt |    1 +
> > > >  tests/generic/475   |    2 +-
> > > >  tests/generic/476   |    2 +-
> > > >  tests/generic/521   |    2 +-
> > > >  tests/generic/522   |    2 +-
> > > >  tests/generic/642   |    2 +-
> > > >  7 files changed, 11 insertions(+), 6 deletions(-)
> > > > 
> > > > 
> > > > diff --git a/check b/check
> > > > index 89e7e7bf20..97c7c4c7d1 100755
> > > > --- a/check
> > > > +++ b/check
> > > > @@ -68,6 +68,7 @@ check options
> > > >      -pvfs2		test PVFS2
> > > >      -tmpfs		test TMPFS
> > > >      -ubifs		test ubifs
> > > > +    -smoketest		run smoke tests for 4min each
> > > 
> > > We have both "smoketest" and "smoke", that's a bit confused :)
> > 
> > We do?  git grep doesn't show anything other than what I added:
> > 
> > $ git grep smoke
> > check:71:    -smoketest         run smoke tests for 4min each
> > check:294:      -smoketest)
> > check:296:              GROUP_LIST="smoketest"
> > doc/group-names.txt:123:smoketest               Simple smoke tests
> > tests/generic/475:15:_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > tests/generic/476:11:_begin_fstest auto rw long_rw stress soak smoketest
> > tests/generic/521:10:_begin_fstest soak long_rw smoketest
> > tests/generic/522:10:_begin_fstest soak long_rw smoketest
> > tests/generic/533:9:# Simple attr smoke tests for user EAs, dereived from generic/097.
> > tests/generic/642:11:_begin_fstest auto soak attr long_rw stress smoketest
> 
> Oh, sorry, my memory is a bit of jumbled ...
> 
> > 
> > > >      -l			line mode diff
> > > >      -udiff		show unified diff (default)
> > > >      -n			show me, do not run tests
> > > > @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
> > > >  		FSTYP=overlay
> > > >  		export OVERLAY=true
> > > >  		;;
> > > > -
> > > > +	-smoketest)
> > > 
> > > Hmm... I'm wondering if it's worth having a specific running option for
> > > someone test group. If each "meaningful" testing way need a specific check
> > > option, the ./check file will be too complicated.
> > > 
> > > If we need some recommended test ways, how about make some separated wrappers
> > > of ./check? For example:
> > > 
> > > # mkdir fstests/runtest/
> > > # cat > fstests/runtest/smoketest <<EOF
> > > export SOAK_DURATION="4m"
> > > ./check -g smoketest
> > > EOF
> > > 
> > > Of course you can write more codes in it.
> > 
> > The goal here was to give casual developers an easy way to run a quick
> > 15 minute exercise *without* having to write wrapper scripts or type
> > all that in every time.  Compare:
> > 
> > $ ./check -smoketest
> > 
> > vs.
> > 
> > $ SOAK_DURATION=4m ./check -g smoketest
> 
> Oh, I don't mean let users write that wrapper, I mean we provide some wrapper
> scripts (to be recommended). E.g.
> 
> # ./runtest/smoaktest
> 
> If we give "smoaktest" a specific run option, what will we do if more people
> want to add more options like that?
> 
> But a wrapper is not an offical running option, it's just a reference which
> can be used directly or can be copied. Then we can have more wrappers from
> each fs expert as reference, to recommend other users how to use fstests
> specially. And I don't need to add options for each of them. What do you think?

I disagree -- this is supposed to be a general smoketest that applies to
any filesystem.  It's easy to discover this option via ./check --help.

Adding wrapper scripts means that now we have to find a separate way to
advertise them and people have to find the wrapper on their own if they
miss the advertising.

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > > +		SOAK_DURATION="4m"
> > > > +		GROUP_LIST="smoketest"
> > > > +		;;
> > > >  	-g)	group=$2 ; shift ;
> > > >  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> > > >  		;;
> > > > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > > > index 1c35a39432..c3dcca3755 100644
> > > > --- a/doc/group-names.txt
> > > > +++ b/doc/group-names.txt
> > > > @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
> > > >  send			btrfs send/receive
> > > >  shrinkfs		decreasing the size of a filesystem
> > > >  shutdown		FS_IOC_SHUTDOWN ioctl
> > > > +smoketest		Simple smoke tests
> > > >  snapshot		btrfs snapshots
> > > >  soak			long running soak tests whose runtime can be controlled
> > > >                          directly by setting the SOAK_DURATION variable
> > > > diff --git a/tests/generic/475 b/tests/generic/475
> > > > index 0cbf5131c2..ce7fe013b1 100755
> > > > --- a/tests/generic/475
> > > > +++ b/tests/generic/475
> > > > @@ -12,7 +12,7 @@
> > > >  # testing efforts.
> > > >  #
> > > >  . ./common/preamble
> > > > -_begin_fstest shutdown auto log metadata eio recoveryloop
> > > > +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > > >  
> > > >  # Override the default cleanup function.
> > > >  _cleanup()
> > > > diff --git a/tests/generic/476 b/tests/generic/476
> > > > index 8e93b73457..b1ae4df4d4 100755
> > > > --- a/tests/generic/476
> > > > +++ b/tests/generic/476
> > > > @@ -8,7 +8,7 @@
> > > >  # bugs in the write path.
> > > >  #
> > > >  . ./common/preamble
> > > > -_begin_fstest auto rw long_rw stress soak
> > > > +_begin_fstest auto rw long_rw stress soak smoketest
> > > >  
> > > >  # Override the default cleanup function.
> > > >  _cleanup()
> > > > diff --git a/tests/generic/521 b/tests/generic/521
> > > > index 22dd31a8ec..0956e50171 100755
> > > > --- a/tests/generic/521
> > > > +++ b/tests/generic/521
> > > > @@ -7,7 +7,7 @@
> > > >  # Long-soak directio fsx test
> > > >  #
> > > >  . ./common/preamble
> > > > -_begin_fstest soak long_rw
> > > > +_begin_fstest soak long_rw smoketest
> > > >  
> > > >  # Import common functions.
> > > >  . ./common/filter
> > > > diff --git a/tests/generic/522 b/tests/generic/522
> > > > index f0cbcb245c..0e4e6009ed 100755
> > > > --- a/tests/generic/522
> > > > +++ b/tests/generic/522
> > > > @@ -7,7 +7,7 @@
> > > >  # Long-soak buffered fsx test
> > > >  #
> > > >  . ./common/preamble
> > > > -_begin_fstest soak long_rw
> > > > +_begin_fstest soak long_rw smoketest
> > > >  
> > > >  # Import common functions.
> > > >  . ./common/filter
> > > > diff --git a/tests/generic/642 b/tests/generic/642
> > > > index eba90903a3..e6a475a8b5 100755
> > > > --- a/tests/generic/642
> > > > +++ b/tests/generic/642
> > > > @@ -8,7 +8,7 @@
> > > >  # bugs in the xattr code.
> > > >  #
> > > >  . ./common/preamble
> > > > -_begin_fstest auto soak attr long_rw stress
> > > > +_begin_fstest auto soak attr long_rw stress smoketest
> > > >  
> > > >  _cleanup()
> > > >  {
> > > > 
> > > 
> > 
> 

Re: [PATCH 2/2] check: generate gcov code coverage reports at the end of each section

[Darrick J. Wong]

On Thu, Jul 20, 2023 at 12:19:16AM +0800, Zorro Lang wrote:
> On Tue, Jul 18, 2023 at 06:11:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Support collecting kernel code coverage information as reported in
> > debugfs.  At the start of each section, we reset the gcov counters;
> > during the section wrapup, we'll collect the kernel gcov data.
> > 
> > If lcov is installed and the kernel source code is available, it will
> > also generate a nice html report.  If a CLI web browser is available, it
> > will also format the html report into text for easy grepping.
> > 
> > This requires the test runner to set REPORT_GCOV=1 explicitly and gcov
> > to be enabled in the kernel.
> > 
> > Cc: tytso@mit.edu
> > Cc: kent.overstreet@linux.dev
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> Hi Darrick,
> 
> Is that possible to split this function from check script to tools/ ?

I don't mind separating it, though I don't see much reason to.  Are you
concerned about ./check growing larger?

--D

> Thanks,
> Zorro
> 
> >  README |    3 ++
> >  check  |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 89 insertions(+)
> > 
> > 
> > diff --git a/README b/README
> > index 9790334db1..ccfdcbe703 100644
> > --- a/README
> > +++ b/README
> > @@ -249,6 +249,9 @@ Kernel/Modules related configuration:
> >     to "forever" and we'll wait forever until the module is gone.
> >   - Set KCONFIG_PATH to specify your preferred location of kernel config
> >     file. The config is used by tests to check if kernel feature is enabled.
> > + - Set REPORT_GCOV to a directory path to make lcov and genhtml generate
> > +   html reports from any gcov code coverage data collected by the kernel.
> > +   If REPORT_GCOV is set to 1, the report will be written to $REPORT_DIR/gcov/.
> >  
> >  Test control:
> >   - Set LOAD_FACTOR to a nonzero positive integer to increase the amount of
> > diff --git a/check b/check
> > index 97c7c4c7d1..3e6f27c653 100755
> > --- a/check
> > +++ b/check
> > @@ -451,6 +451,87 @@ _global_log() {
> >  	fi
> >  }
> >  
> > +GCOV_DIR=/sys/kernel/debug/gcov
> > +
> > +# Find the topmost directories of the .gcno directory hierarchy
> > +_gcov_find_topdirs() {
> > +	find "${GCOV_DIR}/" -name '*.gcno' -printf '%d|%h\n' | \
> > +		sort -g -k 1 | \
> > +		uniq | \
> > +		$AWK_PROG -F '|' 'BEGIN { x = -1 } { if (x < 0) x = $1; if ($1 == x) printf("%s\n", $2);}'
> > +}
> > +
> > +# Generate lcov html report from kernel gcov data if configured
> > +_gcov_generate_report() {
> > +	unset REPORT_GCOV	# don't trigger multiple times if ^C
> > +
> > +	local output_dir="$1"
> > +	test -n "${output_dir}" || return
> > +	test "$output_dir" = "1" && output_dir="$REPORT_DIR/gcov"
> > +
> > +	# Kernel support built in?
> > +	test -d "$GCOV_DIR" || return
> > +
> > +	readarray -t gcno_dirs < <(_gcov_find_topdirs)
> > +	test "${#gcno_dirs[@]}" -gt 0 || return
> > +
> > +	mkdir -p "${output_dir}/"
> > +
> > +	# Collect raw coverage data from the kernel
> > +	readarray -t source_dirs < <(find "${GCOV_DIR}/" -mindepth 1 -maxdepth 1 -type d)
> > +	for dir in "${source_dirs[@]}"; do
> > +		cp -p -R -d -u "${dir}" "${output_dir}/"
> > +	done
> > +
> > +	# If lcov is installed, use it to summarize the gcda data.
> > +	# If it is not installed, there's no point in going forward
> > +	command -v lcov > /dev/null || return
> > +	local lcov=(lcov --exclude 'include*' --capture)
> > +	lcov+=(--output-file "${output_dir}/gcov.report")
> > +	for d in "${gcno_dirs[@]}"; do
> > +		lcov+=(--directory "${d}")
> > +	done
> > +
> > +	# Generate a detailed HTML report from the summary
> > +	local gcov_start_time="$(date --date="${fstests_start_time:-now}")"
> > +	local genhtml=()
> > +	if command -v genhtml > /dev/null; then
> > +		genhtml+=(genhtml -o "${output_dir}/" "${output_dir}/gcov.report")
> > +		genhtml+=(--title "fstests on $(hostname -s) @ ${gcov_start_time}" --legend)
> > +	fi
> > +
> > +	# Try to convert the HTML report summary as text for easier grepping if
> > +	# there's an HTML renderer present
> > +	local totext=()
> > +	test "${#totext[@]}" -eq 0 && \
> > +		command -v lynx &>/dev/null && \
> > +		totext=(lynx -dump "${output_dir}/index.html" -width 120 -nonumbers -nolist)
> > +	test "${#totext[@]}" -eq 0 && \
> > +		command -v links &>/dev/null && \
> > +		totext=(links -dump "${output_dir}/index.html" -width 120)
> > +	test "${#totext[@]}" -eq 0 && \
> > +		command -v elinks &>/dev/null && \
> > +		totext=(elinks -dump "${output_dir}/index.html" --dump-width 120 --no-numbering --no-references)
> > +
> > +	# Analyze kernel data
> > +	"${lcov[@]}" > "${output_dir}/gcov.stdout" 2> "${output_dir}/gcov.stderr"
> > +	test "${#genhtml[@]}" -ne 0 && \
> > +		"${genhtml[@]}" >> "${output_dir}/gcov.stdout" 2>> "${output_dir}/gcov.stderr"
> > +	test "${#totext[@]}" -ne 0 && \
> > +		"${totext[@]}" > "${output_dir}/index.txt" 2>> "${output_dir}/gcov.stderr"
> > +}
> > +
> > +# Reset gcov usage data
> > +_gcov_reset() {
> > +	test -n "${REPORT_GCOV}" || return
> > +
> > +	if [ -w "${GCOV_DIR}/reset" ]; then
> > +		echo 1 > "${GCOV_DIR}/reset"
> > +	else
> > +		unset REPORT_GCOV
> > +	fi
> > +}
> > +
> >  _wrapup()
> >  {
> >  	seq="check"
> > @@ -527,6 +608,10 @@ _wrapup()
> >  					     "${#bad[*]}" "${#notrun[*]}" \
> >  					     "$((sect_stop - sect_start))"
> >  		fi
> > +
> > +		# Generate code coverage report
> > +		test -n "$REPORT_GCOV" && _gcov_generate_report "$REPORT_GCOV"
> > +
> >  		needwrap=false
> >  	fi
> >  
> > @@ -801,6 +886,7 @@ function run_section()
> >  	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
> >  	fi
> >  	echo
> > +	_gcov_reset
> >  	needwrap=true
> >  
> >  	if [ ! -z "$SCRATCH_DEV" ]; then
> > 
> 

Re: [PATCH 2/2] check: generate gcov code coverage reports at the end of each section

[Zorro Lang]

On Wed, Jul 19, 2023 at 07:29:24PM -0700, Darrick J. Wong wrote:
> On Thu, Jul 20, 2023 at 12:19:16AM +0800, Zorro Lang wrote:
> > On Tue, Jul 18, 2023 at 06:11:01PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Support collecting kernel code coverage information as reported in
> > > debugfs.  At the start of each section, we reset the gcov counters;
> > > during the section wrapup, we'll collect the kernel gcov data.
> > > 
> > > If lcov is installed and the kernel source code is available, it will
> > > also generate a nice html report.  If a CLI web browser is available, it
> > > will also format the html report into text for easy grepping.
> > > 
> > > This requires the test runner to set REPORT_GCOV=1 explicitly and gcov
> > > to be enabled in the kernel.
> > > 
> > > Cc: tytso@mit.edu
> > > Cc: kent.overstreet@linux.dev
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > 
> > Hi Darrick,
> > 
> > Is that possible to split this function from check script to tools/ ?
> 
> I don't mind separating it, though I don't see much reason to.  Are you
> concerned about ./check growing larger?

Yeah, a little bit, the check script become smore and more complicated.

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > >  README |    3 ++
> > >  check  |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 89 insertions(+)
> > > 
> > > 
> > > diff --git a/README b/README
> > > index 9790334db1..ccfdcbe703 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -249,6 +249,9 @@ Kernel/Modules related configuration:
> > >     to "forever" and we'll wait forever until the module is gone.
> > >   - Set KCONFIG_PATH to specify your preferred location of kernel config
> > >     file. The config is used by tests to check if kernel feature is enabled.
> > > + - Set REPORT_GCOV to a directory path to make lcov and genhtml generate
> > > +   html reports from any gcov code coverage data collected by the kernel.
> > > +   If REPORT_GCOV is set to 1, the report will be written to $REPORT_DIR/gcov/.
> > >  
> > >  Test control:
> > >   - Set LOAD_FACTOR to a nonzero positive integer to increase the amount of
> > > diff --git a/check b/check
> > > index 97c7c4c7d1..3e6f27c653 100755
> > > --- a/check
> > > +++ b/check
> > > @@ -451,6 +451,87 @@ _global_log() {
> > >  	fi
> > >  }
> > >  
> > > +GCOV_DIR=/sys/kernel/debug/gcov
> > > +
> > > +# Find the topmost directories of the .gcno directory hierarchy
> > > +_gcov_find_topdirs() {
> > > +	find "${GCOV_DIR}/" -name '*.gcno' -printf '%d|%h\n' | \
> > > +		sort -g -k 1 | \
> > > +		uniq | \
> > > +		$AWK_PROG -F '|' 'BEGIN { x = -1 } { if (x < 0) x = $1; if ($1 == x) printf("%s\n", $2);}'
> > > +}
> > > +
> > > +# Generate lcov html report from kernel gcov data if configured
> > > +_gcov_generate_report() {
> > > +	unset REPORT_GCOV	# don't trigger multiple times if ^C
> > > +
> > > +	local output_dir="$1"
> > > +	test -n "${output_dir}" || return
> > > +	test "$output_dir" = "1" && output_dir="$REPORT_DIR/gcov"
> > > +
> > > +	# Kernel support built in?
> > > +	test -d "$GCOV_DIR" || return
> > > +
> > > +	readarray -t gcno_dirs < <(_gcov_find_topdirs)
> > > +	test "${#gcno_dirs[@]}" -gt 0 || return
> > > +
> > > +	mkdir -p "${output_dir}/"
> > > +
> > > +	# Collect raw coverage data from the kernel
> > > +	readarray -t source_dirs < <(find "${GCOV_DIR}/" -mindepth 1 -maxdepth 1 -type d)
> > > +	for dir in "${source_dirs[@]}"; do
> > > +		cp -p -R -d -u "${dir}" "${output_dir}/"
> > > +	done
> > > +
> > > +	# If lcov is installed, use it to summarize the gcda data.
> > > +	# If it is not installed, there's no point in going forward
> > > +	command -v lcov > /dev/null || return
> > > +	local lcov=(lcov --exclude 'include*' --capture)
> > > +	lcov+=(--output-file "${output_dir}/gcov.report")
> > > +	for d in "${gcno_dirs[@]}"; do
> > > +		lcov+=(--directory "${d}")
> > > +	done
> > > +
> > > +	# Generate a detailed HTML report from the summary
> > > +	local gcov_start_time="$(date --date="${fstests_start_time:-now}")"
> > > +	local genhtml=()
> > > +	if command -v genhtml > /dev/null; then
> > > +		genhtml+=(genhtml -o "${output_dir}/" "${output_dir}/gcov.report")
> > > +		genhtml+=(--title "fstests on $(hostname -s) @ ${gcov_start_time}" --legend)
> > > +	fi
> > > +
> > > +	# Try to convert the HTML report summary as text for easier grepping if
> > > +	# there's an HTML renderer present
> > > +	local totext=()
> > > +	test "${#totext[@]}" -eq 0 && \
> > > +		command -v lynx &>/dev/null && \
> > > +		totext=(lynx -dump "${output_dir}/index.html" -width 120 -nonumbers -nolist)
> > > +	test "${#totext[@]}" -eq 0 && \
> > > +		command -v links &>/dev/null && \
> > > +		totext=(links -dump "${output_dir}/index.html" -width 120)
> > > +	test "${#totext[@]}" -eq 0 && \
> > > +		command -v elinks &>/dev/null && \
> > > +		totext=(elinks -dump "${output_dir}/index.html" --dump-width 120 --no-numbering --no-references)
> > > +
> > > +	# Analyze kernel data
> > > +	"${lcov[@]}" > "${output_dir}/gcov.stdout" 2> "${output_dir}/gcov.stderr"
> > > +	test "${#genhtml[@]}" -ne 0 && \
> > > +		"${genhtml[@]}" >> "${output_dir}/gcov.stdout" 2>> "${output_dir}/gcov.stderr"
> > > +	test "${#totext[@]}" -ne 0 && \
> > > +		"${totext[@]}" > "${output_dir}/index.txt" 2>> "${output_dir}/gcov.stderr"
> > > +}
> > > +
> > > +# Reset gcov usage data
> > > +_gcov_reset() {
> > > +	test -n "${REPORT_GCOV}" || return
> > > +
> > > +	if [ -w "${GCOV_DIR}/reset" ]; then
> > > +		echo 1 > "${GCOV_DIR}/reset"
> > > +	else
> > > +		unset REPORT_GCOV
> > > +	fi
> > > +}
> > > +
> > >  _wrapup()
> > >  {
> > >  	seq="check"
> > > @@ -527,6 +608,10 @@ _wrapup()
> > >  					     "${#bad[*]}" "${#notrun[*]}" \
> > >  					     "$((sect_stop - sect_start))"
> > >  		fi
> > > +
> > > +		# Generate code coverage report
> > > +		test -n "$REPORT_GCOV" && _gcov_generate_report "$REPORT_GCOV"
> > > +
> > >  		needwrap=false
> > >  	fi
> > >  
> > > @@ -801,6 +886,7 @@ function run_section()
> > >  	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
> > >  	fi
> > >  	echo
> > > +	_gcov_reset
> > >  	needwrap=true
> > >  
> > >  	if [ ! -z "$SCRATCH_DEV" ]; then
> > > 
> > 
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Wed, Jul 19, 2023 at 07:27:56PM -0700, Darrick J. Wong wrote:
> On Thu, Jul 20, 2023 at 12:11:15AM +0800, Zorro Lang wrote:
> > On Wed, Jul 19, 2023 at 08:29:07AM -0700, Darrick J. Wong wrote:
> > > On Wed, Jul 19, 2023 at 11:10:24PM +0800, Zorro Lang wrote:
> > > > On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > > 
> > > > > Create a "-smoketest" parameter to check that will run generic
> > > > > filesystem smoke testing for five minutes apiece.  Since there are only
> > > > > five smoke tests, this is effectively a 16min super-quick test.
> > > > > 
> > > > > With gcov enabled, running these tests yields about ~75% coverage for
> > > > > iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> > > > > btrfs.  Coverage was about ~65% for the pagecache.
> > > > > 
> > > > > Cc: tytso@mit.edu
> > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > > ---
> > > > >  check               |    6 +++++-
> > > > >  doc/group-names.txt |    1 +
> > > > >  tests/generic/475   |    2 +-
> > > > >  tests/generic/476   |    2 +-
> > > > >  tests/generic/521   |    2 +-
> > > > >  tests/generic/522   |    2 +-
> > > > >  tests/generic/642   |    2 +-
> > > > >  7 files changed, 11 insertions(+), 6 deletions(-)
> > > > > 
> > > > > 
> > > > > diff --git a/check b/check
> > > > > index 89e7e7bf20..97c7c4c7d1 100755
> > > > > --- a/check
> > > > > +++ b/check
> > > > > @@ -68,6 +68,7 @@ check options
> > > > >      -pvfs2		test PVFS2
> > > > >      -tmpfs		test TMPFS
> > > > >      -ubifs		test ubifs
> > > > > +    -smoketest		run smoke tests for 4min each
> > > > 
> > > > We have both "smoketest" and "smoke", that's a bit confused :)
> > > 
> > > We do?  git grep doesn't show anything other than what I added:
> > > 
> > > $ git grep smoke
> > > check:71:    -smoketest         run smoke tests for 4min each
> > > check:294:      -smoketest)
> > > check:296:              GROUP_LIST="smoketest"
> > > doc/group-names.txt:123:smoketest               Simple smoke tests
> > > tests/generic/475:15:_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > > tests/generic/476:11:_begin_fstest auto rw long_rw stress soak smoketest
> > > tests/generic/521:10:_begin_fstest soak long_rw smoketest
> > > tests/generic/522:10:_begin_fstest soak long_rw smoketest
> > > tests/generic/533:9:# Simple attr smoke tests for user EAs, dereived from generic/097.
> > > tests/generic/642:11:_begin_fstest auto soak attr long_rw stress smoketest
> > 
> > Oh, sorry, my memory is a bit of jumbled ...
> > 
> > > 
> > > > >      -l			line mode diff
> > > > >      -udiff		show unified diff (default)
> > > > >      -n			show me, do not run tests
> > > > > @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
> > > > >  		FSTYP=overlay
> > > > >  		export OVERLAY=true
> > > > >  		;;
> > > > > -
> > > > > +	-smoketest)
> > > > 
> > > > Hmm... I'm wondering if it's worth having a specific running option for
> > > > someone test group. If each "meaningful" testing way need a specific check
> > > > option, the ./check file will be too complicated.
> > > > 
> > > > If we need some recommended test ways, how about make some separated wrappers
> > > > of ./check? For example:
> > > > 
> > > > # mkdir fstests/runtest/
> > > > # cat > fstests/runtest/smoketest <<EOF
> > > > export SOAK_DURATION="4m"
> > > > ./check -g smoketest
> > > > EOF
> > > > 
> > > > Of course you can write more codes in it.
> > > 
> > > The goal here was to give casual developers an easy way to run a quick
> > > 15 minute exercise *without* having to write wrapper scripts or type
> > > all that in every time.  Compare:
> > > 
> > > $ ./check -smoketest
> > > 
> > > vs.
> > > 
> > > $ SOAK_DURATION=4m ./check -g smoketest
> > 
> > Oh, I don't mean let users write that wrapper, I mean we provide some wrapper
> > scripts (to be recommended). E.g.
> > 
> > # ./runtest/smoaktest
> > 
> > If we give "smoaktest" a specific run option, what will we do if more people
> > want to add more options like that?
> > 
> > But a wrapper is not an offical running option, it's just a reference which
> > can be used directly or can be copied. Then we can have more wrappers from
> > each fs expert as reference, to recommend other users how to use fstests
> > specially. And I don't need to add options for each of them. What do you think?
> 
> I disagree -- this is supposed to be a general smoketest that applies to
> any filesystem.  It's easy to discover this option via ./check --help.
> 
> Adding wrapper scripts means that now we have to find a separate way to
> advertise them and people have to find the wrapper on their own if they
> miss the advertising.

Hmmm.. OK, but we also have other general test types, e.g. soak test, stress
test, rw test. Should we give each of them a specific option if anyone need
that?

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > > 
> > > --D
> > > 
> > > > Thanks,
> > > > Zorro
> > > > 
> > > > > +		SOAK_DURATION="4m"
> > > > > +		GROUP_LIST="smoketest"
> > > > > +		;;
> > > > >  	-g)	group=$2 ; shift ;
> > > > >  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> > > > >  		;;
> > > > > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > > > > index 1c35a39432..c3dcca3755 100644
> > > > > --- a/doc/group-names.txt
> > > > > +++ b/doc/group-names.txt
> > > > > @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
> > > > >  send			btrfs send/receive
> > > > >  shrinkfs		decreasing the size of a filesystem
> > > > >  shutdown		FS_IOC_SHUTDOWN ioctl
> > > > > +smoketest		Simple smoke tests
> > > > >  snapshot		btrfs snapshots
> > > > >  soak			long running soak tests whose runtime can be controlled
> > > > >                          directly by setting the SOAK_DURATION variable
> > > > > diff --git a/tests/generic/475 b/tests/generic/475
> > > > > index 0cbf5131c2..ce7fe013b1 100755
> > > > > --- a/tests/generic/475
> > > > > +++ b/tests/generic/475
> > > > > @@ -12,7 +12,7 @@
> > > > >  # testing efforts.
> > > > >  #
> > > > >  . ./common/preamble
> > > > > -_begin_fstest shutdown auto log metadata eio recoveryloop
> > > > > +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > > > >  
> > > > >  # Override the default cleanup function.
> > > > >  _cleanup()
> > > > > diff --git a/tests/generic/476 b/tests/generic/476
> > > > > index 8e93b73457..b1ae4df4d4 100755
> > > > > --- a/tests/generic/476
> > > > > +++ b/tests/generic/476
> > > > > @@ -8,7 +8,7 @@
> > > > >  # bugs in the write path.
> > > > >  #
> > > > >  . ./common/preamble
> > > > > -_begin_fstest auto rw long_rw stress soak
> > > > > +_begin_fstest auto rw long_rw stress soak smoketest
> > > > >  
> > > > >  # Override the default cleanup function.
> > > > >  _cleanup()
> > > > > diff --git a/tests/generic/521 b/tests/generic/521
> > > > > index 22dd31a8ec..0956e50171 100755
> > > > > --- a/tests/generic/521
> > > > > +++ b/tests/generic/521
> > > > > @@ -7,7 +7,7 @@
> > > > >  # Long-soak directio fsx test
> > > > >  #
> > > > >  . ./common/preamble
> > > > > -_begin_fstest soak long_rw
> > > > > +_begin_fstest soak long_rw smoketest
> > > > >  
> > > > >  # Import common functions.
> > > > >  . ./common/filter
> > > > > diff --git a/tests/generic/522 b/tests/generic/522
> > > > > index f0cbcb245c..0e4e6009ed 100755
> > > > > --- a/tests/generic/522
> > > > > +++ b/tests/generic/522
> > > > > @@ -7,7 +7,7 @@
> > > > >  # Long-soak buffered fsx test
> > > > >  #
> > > > >  . ./common/preamble
> > > > > -_begin_fstest soak long_rw
> > > > > +_begin_fstest soak long_rw smoketest
> > > > >  
> > > > >  # Import common functions.
> > > > >  . ./common/filter
> > > > > diff --git a/tests/generic/642 b/tests/generic/642
> > > > > index eba90903a3..e6a475a8b5 100755
> > > > > --- a/tests/generic/642
> > > > > +++ b/tests/generic/642
> > > > > @@ -8,7 +8,7 @@
> > > > >  # bugs in the xattr code.
> > > > >  #
> > > > >  . ./common/preamble
> > > > > -_begin_fstest auto soak attr long_rw stress
> > > > > +_begin_fstest auto soak attr long_rw stress smoketest
> > > > >  
> > > > >  _cleanup()
> > > > >  {
> > > > > 
> > > > 
> > > 
> > 
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

On Thu, Jul 20, 2023 at 10:34:33PM +0800, Zorro Lang wrote:
> On Wed, Jul 19, 2023 at 07:27:56PM -0700, Darrick J. Wong wrote:
> > On Thu, Jul 20, 2023 at 12:11:15AM +0800, Zorro Lang wrote:
> > > On Wed, Jul 19, 2023 at 08:29:07AM -0700, Darrick J. Wong wrote:
> > > > On Wed, Jul 19, 2023 at 11:10:24PM +0800, Zorro Lang wrote:
> > > > > On Tue, Jul 18, 2023 at 06:10:56PM -0700, Darrick J. Wong wrote:
> > > > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > > > 
> > > > > > Create a "-smoketest" parameter to check that will run generic
> > > > > > filesystem smoke testing for five minutes apiece.  Since there are only
> > > > > > five smoke tests, this is effectively a 16min super-quick test.
> > > > > > 
> > > > > > With gcov enabled, running these tests yields about ~75% coverage for
> > > > > > iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> > > > > > btrfs.  Coverage was about ~65% for the pagecache.
> > > > > > 
> > > > > > Cc: tytso@mit.edu
> > > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > > > ---
> > > > > >  check               |    6 +++++-
> > > > > >  doc/group-names.txt |    1 +
> > > > > >  tests/generic/475   |    2 +-
> > > > > >  tests/generic/476   |    2 +-
> > > > > >  tests/generic/521   |    2 +-
> > > > > >  tests/generic/522   |    2 +-
> > > > > >  tests/generic/642   |    2 +-
> > > > > >  7 files changed, 11 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > 
> > > > > > diff --git a/check b/check
> > > > > > index 89e7e7bf20..97c7c4c7d1 100755
> > > > > > --- a/check
> > > > > > +++ b/check
> > > > > > @@ -68,6 +68,7 @@ check options
> > > > > >      -pvfs2		test PVFS2
> > > > > >      -tmpfs		test TMPFS
> > > > > >      -ubifs		test ubifs
> > > > > > +    -smoketest		run smoke tests for 4min each
> > > > > 
> > > > > We have both "smoketest" and "smoke", that's a bit confused :)
> > > > 
> > > > We do?  git grep doesn't show anything other than what I added:
> > > > 
> > > > $ git grep smoke
> > > > check:71:    -smoketest         run smoke tests for 4min each
> > > > check:294:      -smoketest)
> > > > check:296:              GROUP_LIST="smoketest"
> > > > doc/group-names.txt:123:smoketest               Simple smoke tests
> > > > tests/generic/475:15:_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > > > tests/generic/476:11:_begin_fstest auto rw long_rw stress soak smoketest
> > > > tests/generic/521:10:_begin_fstest soak long_rw smoketest
> > > > tests/generic/522:10:_begin_fstest soak long_rw smoketest
> > > > tests/generic/533:9:# Simple attr smoke tests for user EAs, dereived from generic/097.
> > > > tests/generic/642:11:_begin_fstest auto soak attr long_rw stress smoketest
> > > 
> > > Oh, sorry, my memory is a bit of jumbled ...
> > > 
> > > > 
> > > > > >      -l			line mode diff
> > > > > >      -udiff		show unified diff (default)
> > > > > >      -n			show me, do not run tests
> > > > > > @@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
> > > > > >  		FSTYP=overlay
> > > > > >  		export OVERLAY=true
> > > > > >  		;;
> > > > > > -
> > > > > > +	-smoketest)
> > > > > 
> > > > > Hmm... I'm wondering if it's worth having a specific running option for
> > > > > someone test group. If each "meaningful" testing way need a specific check
> > > > > option, the ./check file will be too complicated.
> > > > > 
> > > > > If we need some recommended test ways, how about make some separated wrappers
> > > > > of ./check? For example:
> > > > > 
> > > > > # mkdir fstests/runtest/
> > > > > # cat > fstests/runtest/smoketest <<EOF
> > > > > export SOAK_DURATION="4m"
> > > > > ./check -g smoketest
> > > > > EOF
> > > > > 
> > > > > Of course you can write more codes in it.
> > > > 
> > > > The goal here was to give casual developers an easy way to run a quick
> > > > 15 minute exercise *without* having to write wrapper scripts or type
> > > > all that in every time.  Compare:
> > > > 
> > > > $ ./check -smoketest
> > > > 
> > > > vs.
> > > > 
> > > > $ SOAK_DURATION=4m ./check -g smoketest
> > > 
> > > Oh, I don't mean let users write that wrapper, I mean we provide some wrapper
> > > scripts (to be recommended). E.g.
> > > 
> > > # ./runtest/smoaktest
> > > 
> > > If we give "smoaktest" a specific run option, what will we do if more people
> > > want to add more options like that?
> > > 
> > > But a wrapper is not an offical running option, it's just a reference which
> > > can be used directly or can be copied. Then we can have more wrappers from
> > > each fs expert as reference, to recommend other users how to use fstests
> > > specially. And I don't need to add options for each of them. What do you think?
> > 
> > I disagree -- this is supposed to be a general smoketest that applies to
> > any filesystem.  It's easy to discover this option via ./check --help.
> > 
> > Adding wrapper scripts means that now we have to find a separate way to
> > advertise them and people have to find the wrapper on their own if they
> > miss the advertising.
> 
> Hmmm.. OK, but we also have other general test types, e.g. soak test, stress
> test, rw test. Should we give each of them a specific option if anyone need
> that?

If someone wants that, then ok.  The taret audience for this are the
drive-by filesystem patch authors.  IOWs, people who have some small bug
they want to try to fix and want to run a quick test to see if their
change works.

I don't think it's reasonable to expect drive-by'ers to know all that
much about the fstests groups or spend the hours it takes to run -g all.
As a maintainer, I prefer that these folks have done at least a small
taste of QA before they start talking to the lists.

Most people probably won't know which groups they want unless they're
already working with a maintainer on a specific feature.

This isn't intended for experts -- we experts have to know about test
groups and flakey tests and whatnot, and that's fine.

--D

> Thanks,
> Zorro
> 
> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > > 
> > > > --D
> > > > 
> > > > > Thanks,
> > > > > Zorro
> > > > > 
> > > > > > +		SOAK_DURATION="4m"
> > > > > > +		GROUP_LIST="smoketest"
> > > > > > +		;;
> > > > > >  	-g)	group=$2 ; shift ;
> > > > > >  		GROUP_LIST="$GROUP_LIST ${group//,/ }"
> > > > > >  		;;
> > > > > > diff --git a/doc/group-names.txt b/doc/group-names.txt
> > > > > > index 1c35a39432..c3dcca3755 100644
> > > > > > --- a/doc/group-names.txt
> > > > > > +++ b/doc/group-names.txt
> > > > > > @@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
> > > > > >  send			btrfs send/receive
> > > > > >  shrinkfs		decreasing the size of a filesystem
> > > > > >  shutdown		FS_IOC_SHUTDOWN ioctl
> > > > > > +smoketest		Simple smoke tests
> > > > > >  snapshot		btrfs snapshots
> > > > > >  soak			long running soak tests whose runtime can be controlled
> > > > > >                          directly by setting the SOAK_DURATION variable
> > > > > > diff --git a/tests/generic/475 b/tests/generic/475
> > > > > > index 0cbf5131c2..ce7fe013b1 100755
> > > > > > --- a/tests/generic/475
> > > > > > +++ b/tests/generic/475
> > > > > > @@ -12,7 +12,7 @@
> > > > > >  # testing efforts.
> > > > > >  #
> > > > > >  . ./common/preamble
> > > > > > -_begin_fstest shutdown auto log metadata eio recoveryloop
> > > > > > +_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
> > > > > >  
> > > > > >  # Override the default cleanup function.
> > > > > >  _cleanup()
> > > > > > diff --git a/tests/generic/476 b/tests/generic/476
> > > > > > index 8e93b73457..b1ae4df4d4 100755
> > > > > > --- a/tests/generic/476
> > > > > > +++ b/tests/generic/476
> > > > > > @@ -8,7 +8,7 @@
> > > > > >  # bugs in the write path.
> > > > > >  #
> > > > > >  . ./common/preamble
> > > > > > -_begin_fstest auto rw long_rw stress soak
> > > > > > +_begin_fstest auto rw long_rw stress soak smoketest
> > > > > >  
> > > > > >  # Override the default cleanup function.
> > > > > >  _cleanup()
> > > > > > diff --git a/tests/generic/521 b/tests/generic/521
> > > > > > index 22dd31a8ec..0956e50171 100755
> > > > > > --- a/tests/generic/521
> > > > > > +++ b/tests/generic/521
> > > > > > @@ -7,7 +7,7 @@
> > > > > >  # Long-soak directio fsx test
> > > > > >  #
> > > > > >  . ./common/preamble
> > > > > > -_begin_fstest soak long_rw
> > > > > > +_begin_fstest soak long_rw smoketest
> > > > > >  
> > > > > >  # Import common functions.
> > > > > >  . ./common/filter
> > > > > > diff --git a/tests/generic/522 b/tests/generic/522
> > > > > > index f0cbcb245c..0e4e6009ed 100755
> > > > > > --- a/tests/generic/522
> > > > > > +++ b/tests/generic/522
> > > > > > @@ -7,7 +7,7 @@
> > > > > >  # Long-soak buffered fsx test
> > > > > >  #
> > > > > >  . ./common/preamble
> > > > > > -_begin_fstest soak long_rw
> > > > > > +_begin_fstest soak long_rw smoketest
> > > > > >  
> > > > > >  # Import common functions.
> > > > > >  . ./common/filter
> > > > > > diff --git a/tests/generic/642 b/tests/generic/642
> > > > > > index eba90903a3..e6a475a8b5 100755
> > > > > > --- a/tests/generic/642
> > > > > > +++ b/tests/generic/642
> > > > > > @@ -8,7 +8,7 @@
> > > > > >  # bugs in the xattr code.
> > > > > >  #
> > > > > >  . ./common/preamble
> > > > > > -_begin_fstest auto soak attr long_rw stress
> > > > > > +_begin_fstest auto soak attr long_rw stress smoketest
> > > > > >  
> > > > > >  _cleanup()
> > > > > >  {
> > > > > > 
> > > > > 
> > > > 
> > > 
> > 
> 

Re: [PATCH 2/2] check: generate gcov code coverage reports at the end of each section

[Darrick J. Wong]

On Thu, Jul 20, 2023 at 10:24:29PM +0800, Zorro Lang wrote:
> On Wed, Jul 19, 2023 at 07:29:24PM -0700, Darrick J. Wong wrote:
> > On Thu, Jul 20, 2023 at 12:19:16AM +0800, Zorro Lang wrote:
> > > On Tue, Jul 18, 2023 at 06:11:01PM -0700, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > 
> > > > Support collecting kernel code coverage information as reported in
> > > > debugfs.  At the start of each section, we reset the gcov counters;
> > > > during the section wrapup, we'll collect the kernel gcov data.
> > > > 
> > > > If lcov is installed and the kernel source code is available, it will
> > > > also generate a nice html report.  If a CLI web browser is available, it
> > > > will also format the html report into text for easy grepping.
> > > > 
> > > > This requires the test runner to set REPORT_GCOV=1 explicitly and gcov
> > > > to be enabled in the kernel.
> > > > 
> > > > Cc: tytso@mit.edu
> > > > Cc: kent.overstreet@linux.dev
> > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > ---
> > > 
> > > Hi Darrick,
> > > 
> > > Is that possible to split this function from check script to tools/ ?
> > 
> > I don't mind separating it, though I don't see much reason to.  Are you
> > concerned about ./check growing larger?
> 
> Yeah, a little bit, the check script become smore and more complicated.

Ok, I'll do that, and augment the documentation for patch 1/1.

--D

> > 
> > --D
> > 
> > > Thanks,
> > > Zorro
> > > 
> > > >  README |    3 ++
> > > >  check  |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 89 insertions(+)
> > > > 
> > > > 
> > > > diff --git a/README b/README
> > > > index 9790334db1..ccfdcbe703 100644
> > > > --- a/README
> > > > +++ b/README
> > > > @@ -249,6 +249,9 @@ Kernel/Modules related configuration:
> > > >     to "forever" and we'll wait forever until the module is gone.
> > > >   - Set KCONFIG_PATH to specify your preferred location of kernel config
> > > >     file. The config is used by tests to check if kernel feature is enabled.
> > > > + - Set REPORT_GCOV to a directory path to make lcov and genhtml generate
> > > > +   html reports from any gcov code coverage data collected by the kernel.
> > > > +   If REPORT_GCOV is set to 1, the report will be written to $REPORT_DIR/gcov/.
> > > >  
> > > >  Test control:
> > > >   - Set LOAD_FACTOR to a nonzero positive integer to increase the amount of
> > > > diff --git a/check b/check
> > > > index 97c7c4c7d1..3e6f27c653 100755
> > > > --- a/check
> > > > +++ b/check
> > > > @@ -451,6 +451,87 @@ _global_log() {
> > > >  	fi
> > > >  }
> > > >  
> > > > +GCOV_DIR=/sys/kernel/debug/gcov
> > > > +
> > > > +# Find the topmost directories of the .gcno directory hierarchy
> > > > +_gcov_find_topdirs() {
> > > > +	find "${GCOV_DIR}/" -name '*.gcno' -printf '%d|%h\n' | \
> > > > +		sort -g -k 1 | \
> > > > +		uniq | \
> > > > +		$AWK_PROG -F '|' 'BEGIN { x = -1 } { if (x < 0) x = $1; if ($1 == x) printf("%s\n", $2);}'
> > > > +}
> > > > +
> > > > +# Generate lcov html report from kernel gcov data if configured
> > > > +_gcov_generate_report() {
> > > > +	unset REPORT_GCOV	# don't trigger multiple times if ^C
> > > > +
> > > > +	local output_dir="$1"
> > > > +	test -n "${output_dir}" || return
> > > > +	test "$output_dir" = "1" && output_dir="$REPORT_DIR/gcov"
> > > > +
> > > > +	# Kernel support built in?
> > > > +	test -d "$GCOV_DIR" || return
> > > > +
> > > > +	readarray -t gcno_dirs < <(_gcov_find_topdirs)
> > > > +	test "${#gcno_dirs[@]}" -gt 0 || return
> > > > +
> > > > +	mkdir -p "${output_dir}/"
> > > > +
> > > > +	# Collect raw coverage data from the kernel
> > > > +	readarray -t source_dirs < <(find "${GCOV_DIR}/" -mindepth 1 -maxdepth 1 -type d)
> > > > +	for dir in "${source_dirs[@]}"; do
> > > > +		cp -p -R -d -u "${dir}" "${output_dir}/"
> > > > +	done
> > > > +
> > > > +	# If lcov is installed, use it to summarize the gcda data.
> > > > +	# If it is not installed, there's no point in going forward
> > > > +	command -v lcov > /dev/null || return
> > > > +	local lcov=(lcov --exclude 'include*' --capture)
> > > > +	lcov+=(--output-file "${output_dir}/gcov.report")
> > > > +	for d in "${gcno_dirs[@]}"; do
> > > > +		lcov+=(--directory "${d}")
> > > > +	done
> > > > +
> > > > +	# Generate a detailed HTML report from the summary
> > > > +	local gcov_start_time="$(date --date="${fstests_start_time:-now}")"
> > > > +	local genhtml=()
> > > > +	if command -v genhtml > /dev/null; then
> > > > +		genhtml+=(genhtml -o "${output_dir}/" "${output_dir}/gcov.report")
> > > > +		genhtml+=(--title "fstests on $(hostname -s) @ ${gcov_start_time}" --legend)
> > > > +	fi
> > > > +
> > > > +	# Try to convert the HTML report summary as text for easier grepping if
> > > > +	# there's an HTML renderer present
> > > > +	local totext=()
> > > > +	test "${#totext[@]}" -eq 0 && \
> > > > +		command -v lynx &>/dev/null && \
> > > > +		totext=(lynx -dump "${output_dir}/index.html" -width 120 -nonumbers -nolist)
> > > > +	test "${#totext[@]}" -eq 0 && \
> > > > +		command -v links &>/dev/null && \
> > > > +		totext=(links -dump "${output_dir}/index.html" -width 120)
> > > > +	test "${#totext[@]}" -eq 0 && \
> > > > +		command -v elinks &>/dev/null && \
> > > > +		totext=(elinks -dump "${output_dir}/index.html" --dump-width 120 --no-numbering --no-references)
> > > > +
> > > > +	# Analyze kernel data
> > > > +	"${lcov[@]}" > "${output_dir}/gcov.stdout" 2> "${output_dir}/gcov.stderr"
> > > > +	test "${#genhtml[@]}" -ne 0 && \
> > > > +		"${genhtml[@]}" >> "${output_dir}/gcov.stdout" 2>> "${output_dir}/gcov.stderr"
> > > > +	test "${#totext[@]}" -ne 0 && \
> > > > +		"${totext[@]}" > "${output_dir}/index.txt" 2>> "${output_dir}/gcov.stderr"
> > > > +}
> > > > +
> > > > +# Reset gcov usage data
> > > > +_gcov_reset() {
> > > > +	test -n "${REPORT_GCOV}" || return
> > > > +
> > > > +	if [ -w "${GCOV_DIR}/reset" ]; then
> > > > +		echo 1 > "${GCOV_DIR}/reset"
> > > > +	else
> > > > +		unset REPORT_GCOV
> > > > +	fi
> > > > +}
> > > > +
> > > >  _wrapup()
> > > >  {
> > > >  	seq="check"
> > > > @@ -527,6 +608,10 @@ _wrapup()
> > > >  					     "${#bad[*]}" "${#notrun[*]}" \
> > > >  					     "$((sect_stop - sect_start))"
> > > >  		fi
> > > > +
> > > > +		# Generate code coverage report
> > > > +		test -n "$REPORT_GCOV" && _gcov_generate_report "$REPORT_GCOV"
> > > > +
> > > >  		needwrap=false
> > > >  	fi
> > > >  
> > > > @@ -801,6 +886,7 @@ function run_section()
> > > >  	  echo "MOUNT_OPTIONS -- `_scratch_mount_options`"
> > > >  	fi
> > > >  	echo
> > > > +	_gcov_reset
> > > >  	needwrap=true
> > > >  
> > > >  	if [ ! -z "$SCRATCH_DEV" ]; then
> > > > 
> > > 
> > 
> 

[PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a "-smoketest" parameter to check that will run generic
filesystem smoke testing for five minutes apiece.  Since there are only
five smoke tests, this is effectively a 16min super-quick test.

With gcov enabled, running these tests yields about ~75% coverage for
iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
btrfs.  Coverage was about ~65% for the pagecache.

Cc: tytso@mit.edu
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 README              |    9 +++++++++
 check               |    6 +++++-
 doc/group-names.txt |    1 +
 tests/generic/475   |    2 +-
 tests/generic/476   |    2 +-
 tests/generic/521   |    2 +-
 tests/generic/522   |    2 +-
 tests/generic/642   |    2 +-
 8 files changed, 20 insertions(+), 6 deletions(-)


diff --git a/README b/README
index 9790334db1..d4ec73d10d 100644
--- a/README
+++ b/README
@@ -311,6 +311,15 @@ Running tests:
           The TEST and SCRATCH partitions should be pre-formatted
           with another base fs, where the overlay dirs will be created
 
+    - For infrequent filesystem developers who simply want to run a quick test
+      of the most commonly used filesystem functionality, use this command:
+
+          ./check -smoketest <other config options>
+
+      This configures fstests to run five tests to exercise the file I/O,
+      metadata, and crash recovery exercisers for four minutes apiece.  This
+      should complete in approximately 20 minutes.
+
 
     The check script tests the return value of each script, and
     compares the output against the expected output. If the output
diff --git a/check b/check
index 89e7e7bf20..c02e693642 100755
--- a/check
+++ b/check
@@ -68,6 +68,7 @@ check options
     -pvfs2		test PVFS2
     -tmpfs		test TMPFS
     -ubifs		test ubifs
+    -smoketest		run smoke tests for 4min each
     -l			line mode diff
     -udiff		show unified diff (default)
     -n			show me, do not run tests
@@ -290,7 +291,10 @@ while [ $# -gt 0 ]; do
 		FSTYP=overlay
 		export OVERLAY=true
 		;;
-
+	-smoketest)
+		test -z "$SOAK_DURATION" && SOAK_DURATION="4m"
+		GROUP_LIST="smoketest"
+		;;
 	-g)	group=$2 ; shift ;
 		GROUP_LIST="$GROUP_LIST ${group//,/ }"
 		;;
diff --git a/doc/group-names.txt b/doc/group-names.txt
index 1c35a39432..c3dcca3755 100644
--- a/doc/group-names.txt
+++ b/doc/group-names.txt
@@ -118,6 +118,7 @@ selftest		tests with fixed results, used to validate testing setup
 send			btrfs send/receive
 shrinkfs		decreasing the size of a filesystem
 shutdown		FS_IOC_SHUTDOWN ioctl
+smoketest		Simple smoke tests
 snapshot		btrfs snapshots
 soak			long running soak tests whose runtime can be controlled
                         directly by setting the SOAK_DURATION variable
diff --git a/tests/generic/475 b/tests/generic/475
index 0cbf5131c2..ce7fe013b1 100755
--- a/tests/generic/475
+++ b/tests/generic/475
@@ -12,7 +12,7 @@
 # testing efforts.
 #
 . ./common/preamble
-_begin_fstest shutdown auto log metadata eio recoveryloop
+_begin_fstest shutdown auto log metadata eio recoveryloop smoketest
 
 # Override the default cleanup function.
 _cleanup()
diff --git a/tests/generic/476 b/tests/generic/476
index 8e93b73457..b1ae4df4d4 100755
--- a/tests/generic/476
+++ b/tests/generic/476
@@ -8,7 +8,7 @@
 # bugs in the write path.
 #
 . ./common/preamble
-_begin_fstest auto rw long_rw stress soak
+_begin_fstest auto rw long_rw stress soak smoketest
 
 # Override the default cleanup function.
 _cleanup()
diff --git a/tests/generic/521 b/tests/generic/521
index 22dd31a8ec..0956e50171 100755
--- a/tests/generic/521
+++ b/tests/generic/521
@@ -7,7 +7,7 @@
 # Long-soak directio fsx test
 #
 . ./common/preamble
-_begin_fstest soak long_rw
+_begin_fstest soak long_rw smoketest
 
 # Import common functions.
 . ./common/filter
diff --git a/tests/generic/522 b/tests/generic/522
index f0cbcb245c..0e4e6009ed 100755
--- a/tests/generic/522
+++ b/tests/generic/522
@@ -7,7 +7,7 @@
 # Long-soak buffered fsx test
 #
 . ./common/preamble
-_begin_fstest soak long_rw
+_begin_fstest soak long_rw smoketest
 
 # Import common functions.
 . ./common/filter
diff --git a/tests/generic/642 b/tests/generic/642
index eba90903a3..e6a475a8b5 100755
--- a/tests/generic/642
+++ b/tests/generic/642
@@ -8,7 +8,7 @@
 # bugs in the xattr code.
 #
 . ./common/preamble
-_begin_fstest auto soak attr long_rw stress
+_begin_fstest auto soak attr long_rw stress smoketest
 
 _cleanup()
 {

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

On Tue, Jul 25, 2023 at 05:05:24PM -0700, Darrick J. Wong wrote:
> 
> If someone wants that, then ok.  The taret audience for this are the
> drive-by filesystem patch authors.  IOWs, people who have some small bug
> they want to try to fix and want to run a quick test to see if their
> change works.

Zorro,

FYI, the context behind this was a comment I had made to Darrick that
the time necessary to run "-g quick" had been getting longer and
longer, and it might be nice to create a manually curated "-g smoke"
that was good enough for drive-by patch authors.  I was originally
thinking about a cut-down set of tests by selecting a subset of "-g
quick", but Darrick suggested that instead, we just run a very small
set of tests (mostly based on fsstress / fsx) and just run them in a
loop for 4 minutes or so.

We also talked about having a time budget (say, 15 minutes) and then
just dividing 15 time by the number of tests, and just run them in for
a specified soak time, so that the total time is known ahead of time.

To be honest, I was a bit dubious it could be that simple, but that's
where using kcov to show that you get a pretty good code coverage
using something that simple comes from.

> I don't think it's reasonable to expect drive-by'ers to know all that
> much about the fstests groups or spend the hours it takes to run -g all.
> As a maintainer, I prefer that these folks have done at least a small
> taste of QA before they start talking to the lists.

A big problem for the drive-by'ers is that that the top-level xfstests
README file is just plain scary, and has far too many steps for a
drive-by patch author to follow.

What I plan to add to a maintainer-entry-file.rst file for ext4 in the
kernel docs is to tell that drive-by posters that should run
"kvm-xfstests smoke" before submitting a patch, and setting up
kvm-xfstess is dead simple easy:


1)  Install kvm-xfstests --- you only have to run this once

% git clone https://github.com/tytso/xfstests-bld fstests
% cd fstests
% make ; make install

# Optional, if your file system you are developing isn't ext4;
# change f2fs to the file system of your choice
% echo PRIMARY_FSTYPE=f2fs >> ~/.config/kvm-xfstests


2) Build the kernel suitable for use with kvm-xfstests

% cd /path/to/your/kernel
% install-kconfig
% kbuild

3) Run the smoke test --- assuming the cwd is /path/to/your/kernel

(Note: today this runs -g quick, but it would be good if this could be
faster)

% kvm-xfstests smoke 


It's simple, and since the kvm-xfstests script will download a
pre-compiled test appliance image automatically, there's no need to
require the drive-by tester to figure out how compile xfstests with
any of its prerequisites.

And once things are set up, then it's just a matter of running
"kbuild" to build your kernel after you make changes, and running
"kvm-xfstests smoke" to do a quick smoke testing run.

No muss, no fuss, no dirty dishes...   :-)

Cheers,

					- Ted

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Wed, Jul 26, 2023 at 02:01:02AM -0400, Theodore Ts'o wrote:
> On Tue, Jul 25, 2023 at 05:05:24PM -0700, Darrick J. Wong wrote:
> > 
> > If someone wants that, then ok.  The taret audience for this are the
> > drive-by filesystem patch authors.  IOWs, people who have some small bug
> > they want to try to fix and want to run a quick test to see if their
> > change works.
> 
> Zorro,
> 
> FYI, the context behind this was a comment I had made to Darrick that
> the time necessary to run "-g quick" had been getting longer and
> longer, and it might be nice to create a manually curated "-g smoke"
> that was good enough for drive-by patch authors.  I was originally
> thinking about a cut-down set of tests by selecting a subset of "-g
> quick", but Darrick suggested that instead, we just run a very small
> set of tests (mostly based on fsstress / fsx) and just run them in a
> loop for 4 minutes or so.
> 
> We also talked about having a time budget (say, 15 minutes) and then
> just dividing 15 time by the number of tests, and just run them in for
> a specified soak time, so that the total time is known ahead of time.
> 
> To be honest, I was a bit dubious it could be that simple, but that's
> where using kcov to show that you get a pretty good code coverage
> using something that simple comes from.
> 
> > I don't think it's reasonable to expect drive-by'ers to know all that
> > much about the fstests groups or spend the hours it takes to run -g all.
> > As a maintainer, I prefer that these folks have done at least a small
> > taste of QA before they start talking to the lists.
> 
> A big problem for the drive-by'ers is that that the top-level xfstests
> README file is just plain scary, and has far too many steps for a
> drive-by patch author to follow.
> 
> What I plan to add to a maintainer-entry-file.rst file for ext4 in the
> kernel docs is to tell that drive-by posters that should run
> "kvm-xfstests smoke" before submitting a patch, and setting up
> kvm-xfstess is dead simple easy:
> 
> 
> 1)  Install kvm-xfstests --- you only have to run this once
> 
> % git clone https://github.com/tytso/xfstests-bld fstests
> % cd fstests
> % make ; make install
> 
> # Optional, if your file system you are developing isn't ext4;
> # change f2fs to the file system of your choice
> % echo PRIMARY_FSTYPE=f2fs >> ~/.config/kvm-xfstests
> 
> 
> 2) Build the kernel suitable for use with kvm-xfstests
> 
> % cd /path/to/your/kernel
> % install-kconfig
> % kbuild
> 
> 3) Run the smoke test --- assuming the cwd is /path/to/your/kernel
> 
> (Note: today this runs -g quick, but it would be good if this could be
> faster)
> 
> % kvm-xfstests smoke 
> 
> 
> It's simple, and since the kvm-xfstests script will download a
> pre-compiled test appliance image automatically, there's no need to
> require the drive-by tester to figure out how compile xfstests with
> any of its prerequisites.
> 
> And once things are set up, then it's just a matter of running
> "kbuild" to build your kernel after you make changes, and running
> "kvm-xfstests smoke" to do a quick smoke testing run.
> 
> No muss, no fuss, no dirty dishes...   :-)

Hi Ted,

Thanks for this detailed explanation!

Ahaha, I'm just waiting for Darrick wake up, then ask him is there any
requirement/context about this patch. Due to he (looks like) a bit
hurry to push this patch :)

If most of you prefer this way (an ./check option, not a separated wrapper
script), I'm OK with that.

Just recently I'm a bit worry about the ./check code, it's becoming more
and more complex. I hope to separate something from it, but many things
entwined, and growing. Anyway that's another story, I'll look into this
patchset and review it soon.

Thanks,
Zorro

> 
> Cheers,
> 
> 					- Ted
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

On Wed, Jul 26, 2023 at 10:54:41PM +0800, Zorro Lang wrote:
> 
> Ahaha, I'm just waiting for Darrick wake up, then ask him is there any
> requirement/context about this patch. Due to he (looks like) a bit
> hurry to push this patch :)
> 
> If most of you prefer this way (an ./check option, not a separated wrapper
> script), I'm OK with that.

I'm agnostic on that front, since I already *have* my own wrapper
script.  So if we need to do it in the wrapper script, I'm certainly
OK with that.  OTOH, if we think it's a feature which is generally
interesting to multiple developers and/or test wrappers, maybe it
makes sense to push things into the ./check sccript.

So I certainly don't have any objections to adding support to my
/root/runtests.sh so that "{gce,kvm,android}-xfstests smoke" gets ends
up running the moral equivalent of:

SOAK_DURATION=4m ./check -g smoketest

... and adding extra special case support in the check script just for
this use case.  I'm doing enough other stuff in runtests.sh[1] that
it's really not a big deal for me.  :-)

[1] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/root/runtests.sh


More generally, there are some "intresting" hacks --- for example, I
want to be able to run code in between every single test run, and the
way I do it is a big ugly, but effective.  I basically set

LOGGER_PROG to my own special script, gce-logger[2]

[2] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/usr/local/lib/gce-logger

and this allows the user to upload a script which will get run in
between every single individual fstest (e.g., to grab information from
BPF, or grab and reset lockstats, etc.).  This script also updates the
VM metadata so someone can query the VM to find out what test it's
currently running, and the percentage completion for that VM.

I could have asked for extra features in check, but whenever possible
I try to work around it to limit the number of special things just for
my set of wrapper scripts.


> Just recently I'm a bit worry about the ./check code, it's becoming more
> and more complex. I hope to separate something from it, but many things
> entwined, and growing. Anyway that's another story, I'll look into this
> patchset and review it soon.

Well, I don't use the config sections feature at all, because my
wrapper script has a lot more functionality than what you can get with
the config sections, so I just pass in TEST_DEV, SCRATCH_DEV,
MKFS_OPTIONS, etc., via environment variables, and I have my own set
of scripts to set up te test parameters.

So if you were going to simplify things by removing config sections,
*I* wouldn't care.  Enough other people might be using it that
changing the fstests interface for this might raise a lot of
objections from other folks, though.

Cheers,

					- Ted

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

As an aside, while I was testing my updates to the kvm-quickstart[1]
documentation, I timed how long it takes to run "-g quick" for a basic
ext4 file system config with 4k blocks using a desktop NVMe SSD for
the test and scratch devices.

[1] https://github.com/tytso/xfstests-bld/blob/test/Documentation/kvm-quickstart.md

It took 62 minutes, or a little over an hour.  Yowza!  I hadn't
realized that "kvm-xfstests smoke" was now taking that long.  It used
to be that using a slower SSD (an Intel SATA-attached SSD dating from
2008) I could run "-g quick" in 15 minutes.  Clearly, things were a
lot simpler back then.  :-)

Anyway, I definitely need to replace what "kvm-xfstests smoke" does
with something else much more abbrevuated before I start requesting
drive-by patch submitters to run an fstests "smoke test".  Because an
hour isn't it.  Ideally, I'd like to keep it under 10 minutes if at
all possible, but we still want the testing to be likely to detect
most of the sort of simple problems that a drive-by patch submitter
might be likely find....

The fundamental question is how to do get the maximal amount of value
given a limited test budget.

					- Ted

Re: [PATCH 1/2] check: add a -smoketest option

[Darrick J. Wong]

On Wed, Jul 26, 2023 at 09:36:27PM -0400, Theodore Ts'o wrote:
> As an aside, while I was testing my updates to the kvm-quickstart[1]
> documentation, I timed how long it takes to run "-g quick" for a basic
> ext4 file system config with 4k blocks using a desktop NVMe SSD for
> the test and scratch devices.
> 
> [1] https://github.com/tytso/xfstests-bld/blob/test/Documentation/kvm-quickstart.md
> 
> It took 62 minutes, or a little over an hour.  Yowza!  I hadn't
> realized that "kvm-xfstests smoke" was now taking that long.  It used
> to be that using a slower SSD (an Intel SATA-attached SSD dating from
> 2008) I could run "-g quick" in 15 minutes.  Clearly, things were a
> lot simpler back then.  :-)
> 
> Anyway, I definitely need to replace what "kvm-xfstests smoke" does
> with something else much more abbrevuated before I start requesting
> drive-by patch submitters to run an fstests "smoke test".  Because an
> hour isn't it.  Ideally, I'd like to keep it under 10 minutes if at
> all possible, but we still want the testing to be likely to detect
> most of the sort of simple problems that a drive-by patch submitter
> might be likely find....
> 
> The fundamental question is how to do get the maximal amount of value
> given a limited test budget.

Just out of curiosity, if you apply this patch and change kvm-xfstests
smoke to run ./check -smoketest, how long does that actually take on
your infrastructure?

--D

> 					- Ted

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Wed, Jul 26, 2023 at 04:59:30PM -0400, Theodore Ts'o wrote:
> On Wed, Jul 26, 2023 at 10:54:41PM +0800, Zorro Lang wrote:
> > 
> > Ahaha, I'm just waiting for Darrick wake up, then ask him is there any
> > requirement/context about this patch. Due to he (looks like) a bit
> > hurry to push this patch :)
> > 
> > If most of you prefer this way (an ./check option, not a separated wrapper
> > script), I'm OK with that.
> 
> I'm agnostic on that front, since I already *have* my own wrapper
> script.  So if we need to do it in the wrapper script, I'm certainly
> OK with that.  OTOH, if we think it's a feature which is generally
> interesting to multiple developers and/or test wrappers, maybe it
> makes sense to push things into the ./check sccript.
> 
> So I certainly don't have any objections to adding support to my
> /root/runtests.sh so that "{gce,kvm,android}-xfstests smoke" gets ends
> up running the moral equivalent of:
> 
> SOAK_DURATION=4m ./check -g smoketest

Hi Ted,

Now we provide two ways to help to customize testing in fstests:

1)
https://lore.kernel.org/fstests/20230727030529.r4ivp6dmtrht5zo2@zlang-mailbox/T/#mc5cdb59344f4cd681515bf0fab501d7f30f1e263

2)
https://lore.kernel.org/fstests/169033660570.3222210.3010411210438664310.stgit@frogsfrogsfrogs/T/#u

Which one do you like to use? I'd like to hear more review points before I
choose one to merge.

Thanks,
Zorro

> 
> ... and adding extra special case support in the check script just for
> this use case.  I'm doing enough other stuff in runtests.sh[1] that
> it's really not a big deal for me.  :-)
> 
> [1] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/root/runtests.sh
> 
> 
> More generally, there are some "intresting" hacks --- for example, I
> want to be able to run code in between every single test run, and the
> way I do it is a big ugly, but effective.  I basically set
> 
> LOGGER_PROG to my own special script, gce-logger[2]
> 
> [2] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/usr/local/lib/gce-logger
> 
> and this allows the user to upload a script which will get run in
> between every single individual fstest (e.g., to grab information from
> BPF, or grab and reset lockstats, etc.).  This script also updates the
> VM metadata so someone can query the VM to find out what test it's
> currently running, and the percentage completion for that VM.
> 
> I could have asked for extra features in check, but whenever possible
> I try to work around it to limit the number of special things just for
> my set of wrapper scripts.
> 
> 
> > Just recently I'm a bit worry about the ./check code, it's becoming more
> > and more complex. I hope to separate something from it, but many things
> > entwined, and growing. Anyway that's another story, I'll look into this
> > patchset and review it soon.
> 
> Well, I don't use the config sections feature at all, because my
> wrapper script has a lot more functionality than what you can get with
> the config sections, so I just pass in TEST_DEV, SCRATCH_DEV,
> MKFS_OPTIONS, etc., via environment variables, and I have my own set
> of scripts to set up te test parameters.
> 
> So if you were going to simplify things by removing config sections,
> *I* wouldn't care.  Enough other people might be using it that
> changing the fstests interface for this might raise a lot of
> objections from other folks, though.
> 
> Cheers,
> 
> 					- Ted
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

On Thu, Jul 27, 2023 at 11:25:37AM +0800, Zorro Lang wrote:
> > SOAK_DURATION=4m ./check -g smoketest
> 
> Now we provide two ways to help to customize testing in fstests:
> 
> 1)
> https://lore.kernel.org/fstests/20230727030529.r4ivp6dmtrht5zo2@zlang-mailbox/T/#mc5cdb59344f4cd681515bf0fab501d7f30f1e263
> 
> 2)
> https://lore.kernel.org/fstests/169033660570.3222210.3010411210438664310.stgit@frogsfrogsfrogs/T/#u
> 
> Which one do you like to use? I'd like to hear more review points before I
> choose one to merge.

(1) is the "./check -t smoketest" option, and it provides a more
generic way of adding new templates.  On the positive side it allows
more of this kind of simple "configuration" style options where "-t
smoketest" is essentially syntactic sugar for:

	SOAK_DURATION=${SOAK_DURATION:-4m} ./check -g smoketest"

The potential disadvantage of (1) is that it seems like extra
complexity for what is really simple.


(2) is "./check -smoketest" option.  Its advantage is that it might
easier for a drive-by patcher to type.  The disadvantage is that it's
adding Yet Another Option to the ./check script.

I also will note that we have some "long options" which use a single
hypen (e.g., -overlay and -udiff) but newer "long options" seem to use
the double hypehn approach (e.g., --exact-order and --large-fs).  My
personal preference is for the newer GNU getopt style of using double
hyphens, but the fact that we have both types of long options
is... unfortunate.


I guess I have a slight preference for (1), but I'm really not sure
either is really necessary.  My view is that for a drive-by tester,
trying to set up xfstests is Too Hard.  So the reality is they will be
using some kind of wrapper script --- either one that they've written
for their own, such as what Darrick (and I assume other XFS developers
have their own), or they're using something like kdevops or
kvm-xfstests.

From *my* perspective, I have absolutely *no* problem with having my
wrapper script use:

	SOACK_DURATION=4m ./check -g smoketest

because I only have to do it once, and no end-user is ever going to
see it.  They will just use "kvm-xfstests smoke", and all of the magic
will be hidden from them.

The main advantage of having some kind of "official" top-level way of
specifying the smoke test is that it makes it more likely that
different wrapper scripts will converge on the same kind of smoke
test, and it becomes easier for fstests developers to communicate with
each other because the concept of what a "smoke test" is has been well
defined in the fstests source code.  And for that purpose, I think the
"./check -t smoketest" approach works just fine.

But really, I can live with either.   :-)

Cheers,

						- Ted

Re: [PATCH 1/2] check: add a -smoketest option

[Zorro Lang]

On Thu, Jul 27, 2023 at 10:33:26AM -0400, Theodore Ts'o wrote:
> On Thu, Jul 27, 2023 at 11:25:37AM +0800, Zorro Lang wrote:
> > > SOAK_DURATION=4m ./check -g smoketest
> > 
> > Now we provide two ways to help to customize testing in fstests:
> > 
> > 1)
> > https://lore.kernel.org/fstests/20230727030529.r4ivp6dmtrht5zo2@zlang-mailbox/T/#mc5cdb59344f4cd681515bf0fab501d7f30f1e263
> > 
> > 2)
> > https://lore.kernel.org/fstests/169033660570.3222210.3010411210438664310.stgit@frogsfrogsfrogs/T/#u
> > 
> > Which one do you like to use? I'd like to hear more review points before I
> > choose one to merge.
> 
> (1) is the "./check -t smoketest" option, and it provides a more
> generic way of adding new templates.  On the positive side it allows
> more of this kind of simple "configuration" style options where "-t
> smoketest" is essentially syntactic sugar for:
> 
> 	SOAK_DURATION=${SOAK_DURATION:-4m} ./check -g smoketest"
> 
> The potential disadvantage of (1) is that it seems like extra
> complexity for what is really simple.
> 
> 
> (2) is "./check -smoketest" option.  Its advantage is that it might
> easier for a drive-by patcher to type.  The disadvantage is that it's
> adding Yet Another Option to the ./check script.
> 
> I also will note that we have some "long options" which use a single
> hypen (e.g., -overlay and -udiff) but newer "long options" seem to use
> the double hypehn approach (e.g., --exact-order and --large-fs).  My
> personal preference is for the newer GNU getopt style of using double
> hyphens, but the fact that we have both types of long options
> is... unfortunate.

Yeah, I'd like to tidy the ./check, include the option names. But change the
check option format will affect many users, cause most of their scripts go
wrong suddently, then they need to check and use new option format. That's
why I still not touch this part.

> 
> 
> I guess I have a slight preference for (1), but I'm really not sure
> either is really necessary.  My view is that for a drive-by tester,
> trying to set up xfstests is Too Hard.  So the reality is they will be
> using some kind of wrapper script --- either one that they've written
> for their own, such as what Darrick (and I assume other XFS developers
> have their own), or they're using something like kdevops or
> kvm-xfstests.

Sure, you're right. fstests can be used to do a simple test, but for regular
test, a wrapper is needed. Darrick has his wrapper, Dave might has his wrapper
too. My team also has our wrapper, (we also have wrappers to run ltp and others).
Different users might have different testing environment, so they might build
different wrappers to connect fstests with their own environment/requirement.

So I'd like to keep fstests as simple underlying test cases, provide basic
testing functions to anyone who would like to run it, try to not limit much
things. But I'd like to let fstests provides more help to each of your testing
requirement. That's my current crude thought :-P

> 
> From *my* perspective, I have absolutely *no* problem with having my
> wrapper script use:
> 
> 	SOACK_DURATION=4m ./check -g smoketest
> 
> because I only have to do it once, and no end-user is ever going to
> see it.  They will just use "kvm-xfstests smoke", and all of the magic
> will be hidden from them.
> 
> The main advantage of having some kind of "official" top-level way of
> specifying the smoke test is that it makes it more likely that
> different wrapper scripts will converge on the same kind of smoke
> test, and it becomes easier for fstests developers to communicate with
> each other because the concept of what a "smoke test" is has been well
> defined in the fstests source code.  And for that purpose, I think the
> "./check -t smoketest" approach works just fine.

OK, thanks for your reply. I'll double check with Darrick, then merge
one of them :)

Thanks,
Zorro

> 
> But really, I can live with either.   :-)
> 
> Cheers,
> 
> 						- Ted
> 

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

On Tue, Jul 25, 2023 at 06:56:45PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a "-smoketest" parameter to check that will run generic
> filesystem smoke testing for five minutes apiece.  Since there are only
> five smoke tests, this is effectively a 16min super-quick test.

The code is setting SOAK_DURATION to 4 minutes, not 5 minutes.
However, when I ran the moral equivalent:

    kvm-xfstests --soak-duration 4m --fail-loop-count 0 -c ext4/4k \
        generic/475 generic/476 generic/521 generic/522 generic/642

It overall took 17 minutes to run, with just under a minute of test
infrastructure overhead (in the check script and my wrapper scripts),
with the actual test time as follows:

ext4/4k: 5 tests, 975 seconds
  generic/475  Pass     242s
  generic/476  Pass     244s
  generic/521  Pass     241s
  generic/522  Pass     241s
  generic/642  Pass     7s
Totals: 5 tests, 0 skipped, 0 failures, 0 errors, 975s

The time which generic/642 ran was surprising so I took a closer look.
It does claim to be in group "soak", and it even tries to canonicalize
SOAK_DURATION (I'm not sure why, since the check script does this
already).  But generic/642 doesn't seem to use SOAK_DURATION.  It does
caculate a DURATION value, but it doesn't actually use SOAK_DURATION.

So that sounds like a bug in the generic/642 test?

There was also a bug xfstests's "make install" in that it doesn't
actually install src/soak_duration.awk, but I'll send that a patch
fixing that under separate cover.

Darrick -- suppose changed the SOAK_DURATION down to 2 minutes?  How
much do you think that would materially affect the code coverage
metrics, and the overall effectiveness of the smoke test?  If we get
generci/642 to honor SOAK_DURATION, using an overall 2 minute soak for
each test would translate to the smoke test taking about 13 minutes,
which would be great from a drive-by patch submitter perspective.

      	       	     	    	     	   - Ted

> With gcov enabled, running these tests yields about ~75% coverage for
> iomap and ~60% for xfs; or ~50% for ext4 and ~75% for ext4; and ~45% for
> btrfs.  Coverage was about ~65% for the pagecache.

Re: [PATCH 1/2] check: add a -smoketest option

[Theodore Ts'o]

On Thu, Jul 27, 2023 at 11:30:46PM +0800, Zorro Lang wrote:
> 
> Yeah, I'd like to tidy the ./check, include the option names. But change the
> check option format will affect many users, cause most of their scripts go
> wrong suddently, then they need to check and use new option format. That's
> why I still not touch this part.

What if we add support for the double-hyphen variant for those options
that use a syingle-hyphen, and then wait say, a year or so, and then
add a deprecation warning to the single hyphen options, and then a
year latter drop support the single-hypen variant?

I can send a patch for the first part of the process if you think that
makes sense....

      	     	       	    	   - Ted