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