diff mbox series

[blktests] block/034: Test memory is released by null-blk driver with memory_backed=1

Message ID 20230607114836.9779-1-nj.shetty@samsung.com (mailing list archive)
State New, archived
Headers show
Series [blktests] block/034: Test memory is released by null-blk driver with memory_backed=1 | expand

Commit Message

Nitesh Shetty June 7, 2023, 11:48 a.m. UTC
This tests memory leak, by loading/unloading nullblk driver.
Steps:
	1. Load nullblk driver with memory_backed=1
	2. "dd" of 50M
	3. Unload null-blk driver
We do it for 5 iterations to avoid any noise.

Commit 8cfb98196cceec35416041c6b91212d2b99392e4 fixes issue in kernel

Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
---
 tests/block/034     | 60 +++++++++++++++++++++++++++++++++++++++++++++
 tests/block/034.out |  2 ++
 2 files changed, 62 insertions(+)
 create mode 100644 tests/block/034
 create mode 100644 tests/block/034.out

Comments

Shin'ichiro Kawasaki June 14, 2023, 10:24 a.m. UTC | #1
On Jun 07, 2023 / 17:18, Nitesh Shetty wrote:
> This tests memory leak, by loading/unloading nullblk driver.
> Steps:
> 	1. Load nullblk driver with memory_backed=1
> 	2. "dd" of 50M
> 	3. Unload null-blk driver
> We do it for 5 iterations to avoid any noise.
> 
> Commit 8cfb98196cceec35416041c6b91212d2b99392e4 fixes issue in kernel

Hi Nitesh, thanks for the patch. Good to have this test case. I ran it
and confirmed it detects the issue in stable manner. Please find some
comments in line for improvements, and see if they are reasonable.

> 
> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
> ---
>  tests/block/034     | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/block/034.out |  2 ++
>  2 files changed, 62 insertions(+)
>  create mode 100644 tests/block/034
>  create mode 100644 tests/block/034.out
> 
> diff --git a/tests/block/034 b/tests/block/034
> new file mode 100644
> index 0000000..dc4f447
> --- /dev/null
> +++ b/tests/block/034
> @@ -0,0 +1,60 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2023 Nitesh Shetty
> +#
> +# Check memory leak when null_blk driver is loaded with memory_backed=1
> +
> +. tests/block/rc
> +. common/null_blk
> +
> +DESCRIPTION="load/unload null_blk memory_backed=1 and run dd to check memleak"

Nit: the line is a bit long. I suggest to remove the part "and run dd".

> +TIMED=1

The line above is not appropriate since this test case does not refer $TIMEOUT.
Instead, I suggest to add "QUICK=1", since it takes only a few seconds to run.

> +
> +requires() {
> +	_have_null_blk

Instead of the line above, I suggest "_have_module" null_blk. It will explicitly
tell that this test case requires loadable null_blk. It will avoid execution of
the test() function when null_blk is built-in.

> +	_have_module_param null_blk memory_backed
> +	_have_program dd
> +}
> +
> +run_nullblk_dd() {
> +	if ! _init_null_blk memory_backed=1; then
> +		echo "Loading null_blk failed"
> +		return 1
> +	fi
> +	dd if=/dev/urandom of=/dev/nullb0 oflag=direct bs=1M count="$1" > \
> +		/dev/null 2>&1
> +	_exit_null_blk
> +}
> +
> +free_memory() {
> +	mem=$(sed -n 's/^MemFree:[[:blank:]]*\([0-9]*\)[[:blank:]]*kB$/\1/p' \
> +		/proc/meminfo)
> +	echo "$mem"

Nit: I think the mem variable assignment and echo command are not required. Just
executing the sed command is enough.

> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	mem_leak=0
> +	size=50

Nit: local variable declarations will help to read code a bit, like:

       local mem_leak=0 size=50
       local i mem_start mem_end mem_used

> +	for ((i = 0; i < 5; i++)); do
> +		mem_start=$(free_memory)
> +		run_nullblk_dd $size
> +		mem_end=$(free_memory)
> +
> +		mem_used=$((((mem_start - mem_end)) / 1024))

Number of parenetheses can be reduced:

                mem_used=$(((mem_start - mem_end) / 1024))

> +		# -10MB to account for some randomness in freeing by some
> +		# simultaneous process
> +		if [ $mem_used -ge $((size - 10)) ]; then

Nit: Bash arithmetic could be easier to read:

                if ((mem_used >= size - 10)); then

> +			mem_leak=$((mem_leak + 1))
> +		fi
> +	done
> +
> +	# There might be possibilty of some random process freeing up memory at
> +	# same time nullblk is unloaded.
> +	# we consider 3/5 times to be positive.
> +	if [ $mem_leak -gt 3 ]; then

Nit: same here:

        if ((mem_leak > 3)); then

> +		echo "Memleak: Memory is not freed by null-blk driver"
> +	fi
> +	echo "Test complete"
> +}
> diff --git a/tests/block/034.out b/tests/block/034.out
> new file mode 100644
> index 0000000..a916dd8
> --- /dev/null
> +++ b/tests/block/034.out
> @@ -0,0 +1,2 @@
> +Running block/034
> +Test complete
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/tests/block/034 b/tests/block/034
new file mode 100644
index 0000000..dc4f447
--- /dev/null
+++ b/tests/block/034
@@ -0,0 +1,60 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2023 Nitesh Shetty
+#
+# Check memory leak when null_blk driver is loaded with memory_backed=1
+
+. tests/block/rc
+. common/null_blk
+
+DESCRIPTION="load/unload null_blk memory_backed=1 and run dd to check memleak"
+TIMED=1
+
+requires() {
+	_have_null_blk
+	_have_module_param null_blk memory_backed
+	_have_program dd
+}
+
+run_nullblk_dd() {
+	if ! _init_null_blk memory_backed=1; then
+		echo "Loading null_blk failed"
+		return 1
+	fi
+	dd if=/dev/urandom of=/dev/nullb0 oflag=direct bs=1M count="$1" > \
+		/dev/null 2>&1
+	_exit_null_blk
+}
+
+free_memory() {
+	mem=$(sed -n 's/^MemFree:[[:blank:]]*\([0-9]*\)[[:blank:]]*kB$/\1/p' \
+		/proc/meminfo)
+	echo "$mem"
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	mem_leak=0
+	size=50
+	for ((i = 0; i < 5; i++)); do
+		mem_start=$(free_memory)
+		run_nullblk_dd $size
+		mem_end=$(free_memory)
+
+		mem_used=$((((mem_start - mem_end)) / 1024))
+		# -10MB to account for some randomness in freeing by some
+		# simultaneous process
+		if [ $mem_used -ge $((size - 10)) ]; then
+			mem_leak=$((mem_leak + 1))
+		fi
+	done
+
+	# There might be possibilty of some random process freeing up memory at
+	# same time nullblk is unloaded.
+	# we consider 3/5 times to be positive.
+	if [ $mem_leak -gt 3 ]; then
+		echo "Memleak: Memory is not freed by null-blk driver"
+	fi
+	echo "Test complete"
+}
diff --git a/tests/block/034.out b/tests/block/034.out
new file mode 100644
index 0000000..a916dd8
--- /dev/null
+++ b/tests/block/034.out
@@ -0,0 +1,2 @@ 
+Running block/034
+Test complete