Re: [PATCH v29 12/13] mm/damon: Add user space selftests

[Maximilian Heyne]

On Thu, 20 May 2021 07:56:28 +0000 SeongJae Park <sj38.park@xxxxxxxxx> wrote:

> From: SeongJae Park <sjpark@xxxxxxxxx>
> 
> This commit adds a simple user space tests for DAMON.  The tests are
> using kselftest framework.
> 
> Signed-off-by: SeongJae Park <sjpark@xxxxxxxxx>
> ---
>  tools/testing/selftests/damon/Makefile        |  7 ++
>  .../selftests/damon/_chk_dependency.sh        | 28 ++++++
>  .../testing/selftests/damon/debugfs_attrs.sh  | 98 +++++++++++++++++++
>  3 files changed, 133 insertions(+)
>  create mode 100644 tools/testing/selftests/damon/Makefile
>  create mode 100644 tools/testing/selftests/damon/_chk_dependency.sh
>  create mode 100755 tools/testing/selftests/damon/debugfs_attrs.sh
> 
> diff --git a/tools/testing/selftests/damon/Makefile b/tools/testing/selftests/damon/Makefile
> new file mode 100644
> index 000000000000..8a3f2cd9fec0
> --- /dev/null
> +++ b/tools/testing/selftests/damon/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Makefile for damon selftests
> +
> +TEST_FILES = _chk_dependency.sh
> +TEST_PROGS = debugfs_attrs.sh
> +
> +include ../lib.mk
> diff --git a/tools/testing/selftests/damon/_chk_dependency.sh b/tools/testing/selftests/damon/_chk_dependency.sh
> new file mode 100644
> index 000000000000..e090836c2bf7
> --- /dev/null
> +++ b/tools/testing/selftests/damon/_chk_dependency.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +DBGFS=/sys/kernel/debug/damon
> +
> +if [ $EUID -ne 0 ];
> +then
> +	echo "Run as root"
> +	exit $ksft_skip
> +fi
> +
> +if [ ! -d $DBGFS ]
> +then
> +	echo "$DBGFS not found"
> +	exit $ksft_skip
> +fi
> +
> +for f in attrs target_ids monitor_on
> +do
> +	if [ ! -f "$DBGFS/$f" ]
> +	then
> +		echo "$f not found"
> +		exit 1
> +	fi
> +done
> diff --git a/tools/testing/selftests/damon/debugfs_attrs.sh b/tools/testing/selftests/damon/debugfs_attrs.sh
> new file mode 100755
> index 000000000000..4a8ab4910ee4
> --- /dev/null
> +++ b/tools/testing/selftests/damon/debugfs_attrs.sh
> @@ -0,0 +1,98 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +source ./_chk_dependency.sh
> +
> +# Test attrs file
> +file="$DBGFS/attrs"
> +
> +ORIG_CONTENT=$(cat $file)

Missing quotes around $file. Can you run shellcheck on this code and fix all
reportings, please?

> +
> +echo 1 2 3 4 5 > $file
> +if [ $? -ne 0 ]
> +then
> +	echo "$file write failed"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +echo 1 2 3 4 > $file
> +if [ $? -eq 0 ]
> +then
> +	echo "$file write success (should failed)"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +CONTENT=$(cat $file)
> +if [ "$CONTENT" != "1 2 3 4 5" ]
> +then
> +	echo "$file not written"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi

I'd add test cases for the contents written to the attrs, like checking that
input min_nr_regions is actually smaller than the input max_nr_regions values.

> +
> +echo $ORIG_CONTENT > $file
> +
> +# Test target_ids file
> +file="$DBGFS/target_ids"
> +
> +ORIG_CONTENT=$(cat $file)
> +
> +echo "1 2 3 4" > $file
> +if [ $? -ne 0 ]
> +then
> +	echo "$file write fail"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +echo "1 2 abc 4" > $file
> +if [ $? -ne 0 ]
> +then
> +	echo "$file write fail"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi

I've seen this construct more than once. Any chance to refactor this code? Or is
this selftest not expected to grow in the future?

> +
> +CONTENT=$(cat $file)
> +if [ "$CONTENT" != "1 2" ]
> +then
> +	echo "$file not written"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +echo abc 2 3 > $file
> +if [ $? -ne 0 ]
> +then
> +	echo "$file wrong value write fail"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +if [ ! -z "$(cat $file)" ]
> +then
> +	echo "$file not cleared"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +echo > $file
> +if [ $? -ne 0 ]
> +then
> +	echo "$file init fail"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +if [ ! -z "$(cat $file)" ]
> +then
> +	echo "$file not initialized"
> +	echo $ORIG_CONTENT > $file
> +	exit 1
> +fi
> +
> +echo $ORIG_CONTENT > $file
> +
> +echo "PASS"
> -- 
> 2.17.1
> 
> 
> 
> 



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879