[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210611135737.104838-1-mheyne@amazon.de>
Date: Fri, 11 Jun 2021 13:57:37 +0000
From: Maximilian Heyne <mheyne@...zon.de>
To: SeongJae Park <sj38.park@...il.com>
CC: <akpm@...ux-foundation.org>, SeongJae Park <sjpark@...zon.de>,
<Jonathan.Cameron@...wei.com>, <acme@...nel.org>,
<alexander.shishkin@...ux.intel.com>, <amit@...nel.org>,
<benh@...nel.crashing.org>, <brendanhiggins@...gle.com>,
<corbet@....net>, <david@...hat.com>, <dwmw@...zon.com>,
<elver@...gle.com>, <fan.du@...el.com>, <foersleo@...zon.de>,
<greg@...ah.com>, <gthelen@...gle.com>,
<guoju.fgj@...baba-inc.com>, <mgorman@...e.de>,
<minchan@...nel.org>, <mingo@...hat.com>, <namhyung@...nel.org>,
<peterz@...radead.org>, <riel@...riel.com>, <rientjes@...gle.com>,
<rostedt@...dmis.org>, <rppt@...nel.org>, <shakeelb@...gle.com>,
<shuah@...nel.org>, <snu@...zon.de>, <vbabka@...e.cz>,
<vdavydov.dev@...il.com>, <zgf574564920@...il.com>,
<linux-damon@...zon.com>, <linux-mm@...ck.org>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v29 12/13] mm/damon: Add user space selftests
On Thu, 20 May 2021 07:56:28 +0000 SeongJae Park <sj38.park@...il.com> wrote:
> From: SeongJae Park <sjpark@...zon.de>
>
> This commit adds a simple user space tests for DAMON. The tests are
> using kselftest framework.
>
> Signed-off-by: SeongJae Park <sjpark@...zon.de>
> ---
> 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
Powered by blists - more mailing lists