* [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
@ 2020-03-20 22:24 Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-03-20 22:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, Chaitanya Kulkarni, Bart Van Assche
Hi Omar,
Please consider this patch series for the upstream blktests repository.
Thanks,
Bart.
Changes compared to v2:
- Addressed Omar's review comments.
Changes between v1 and v2:
- Added three patches that refactor null_blk loading, unloading and
configuration (Chaitanya).
Bart Van Assche (4):
Make _exit_null_blk remove all null_blk device instances
Use _{init,exit}_null_blk instead of open-coding these functions
Introduce the function _configure_null_blk()
Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
common/multipath-over-rdma | 29 ++++++++------------------
common/null_blk | 21 ++++++++++++++++++-
tests/block/022 | 3 ---
tests/block/029 | 17 ++-------------
tests/block/030 | 42 ++++++++++++++++++++++++++++++++++++++
tests/block/030.out | 1 +
tests/nvmeof-mp/rc | 2 +-
7 files changed, 74 insertions(+), 41 deletions(-)
create mode 100755 tests/block/030
create mode 100644 tests/block/030.out
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances
2020-03-20 22:24 [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
@ 2020-03-20 22:24 ` Bart Van Assche
2020-03-23 11:09 ` Daniel Wagner
2020-03-20 22:24 ` [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions Bart Van Assche
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-20 22:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, Chaitanya Kulkarni, Bart Van Assche
Instead of making every test remove null_blk device instances before calling
_exit_null_blk(), move the null_blk device instance removal code into
_exit_null_blk().
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
common/null_blk | 7 ++++++-
tests/block/022 | 3 ---
tests/block/029 | 1 -
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/common/null_blk b/common/null_blk
index 2e300c20bbc7..a4140e365955 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -8,11 +8,15 @@ _have_null_blk() {
_have_modules null_blk
}
-_init_null_blk() {
+_remove_null_blk_devices() {
if [[ -d /sys/kernel/config/nullb ]]; then
find /sys/kernel/config/nullb -mindepth 1 -maxdepth 1 \
-type d -delete
fi
+}
+
+_init_null_blk() {
+ _remove_null_blk_devices
local zoned=""
if (( RUN_FOR_ZONED )); then zoned="zoned=1"; fi
@@ -26,6 +30,7 @@ _init_null_blk() {
}
_exit_null_blk() {
+ _remove_null_blk_devices
udevadm settle
modprobe -r null_blk
}
diff --git a/tests/block/022 b/tests/block/022
index 1404aacef295..b2c53e266d81 100755
--- a/tests/block/022
+++ b/tests/block/022
@@ -50,9 +50,6 @@ test() {
wait $pid1
} 2>/dev/null
- rmdir /sys/kernel/config/nullb/1
- rmdir /sys/kernel/config/nullb/0
-
_exit_null_blk
echo "Test complete"
}
diff --git a/tests/block/029 b/tests/block/029
index d298bac8db5c..0d521edb0cf6 100755
--- a/tests/block/029
+++ b/tests/block/029
@@ -58,7 +58,6 @@ test() {
else
echo "Skipping test because $sq cannot be modified" >>"$FULL"
fi
- rmdir /sys/kernel/config/nullb/nullb0
_exit_null_blk
echo Passed
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions
2020-03-20 22:24 [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances Bart Van Assche
@ 2020-03-20 22:24 ` Bart Van Assche
2020-03-23 11:13 ` Daniel Wagner
2020-03-20 22:24 ` [PATCH blktests v3 3/4] Introduce the function _configure_null_blk() Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
3 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-20 22:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, Chaitanya Kulkarni, Bart Van Assche
This patch reduces code duplication.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
common/multipath-over-rdma | 12 +++---------
tests/nvmeof-mp/rc | 2 +-
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index 40efc4b3aa2e..a56e7a8269db 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -5,6 +5,7 @@
# Functions and global variables used by both the srp and nvmeof-mp tests.
. common/shellcheck
+. common/null_blk
debug=
filesystem_type=ext4
@@ -634,13 +635,6 @@ configure_null_blk() {
ls -l /dev/nullb* &>>"$FULL"
}
-unload_null_blk() {
- local d
-
- for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
- unload_module null_blk
-}
-
setup_rdma() {
start_soft_rdma
(
@@ -662,7 +656,7 @@ teardown_uncond() {
rm -f /etc/multipath.conf
stop_target
stop_soft_rdma
- unload_null_blk
+ _exit_null_blk
}
teardown() {
@@ -698,7 +692,7 @@ setup_test() {
[ -e "/sys/module/$m" ] || modprobe "$m" || return $?
done
- modprobe null_blk nr_devices=0 || return $?
+ _init_null_blk nr_devices=0 || return $?
configure_null_blk || return $?
diff --git a/tests/nvmeof-mp/rc b/tests/nvmeof-mp/rc
index 1fd631445921..136163bc73ad 100755
--- a/tests/nvmeof-mp/rc
+++ b/tests/nvmeof-mp/rc
@@ -278,7 +278,7 @@ stop_nvme_target() {
)
unload_module nvmet_rdma &&
unload_module nvmet &&
- unload_null_blk
+ _exit_null_blk
}
start_target() {
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH blktests v3 3/4] Introduce the function _configure_null_blk()
2020-03-20 22:24 [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions Bart Van Assche
@ 2020-03-20 22:24 ` Bart Van Assche
2020-03-23 11:19 ` Daniel Wagner
2020-03-20 22:24 ` [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
3 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-20 22:24 UTC (permalink / raw)
To: Omar Sandoval
Cc: linux-block, Chaitanya Kulkarni, Bart Van Assche, Chaitanya Kulkarni
Introduce a function for creating a null_blk device instance through
configfs.
Suggested-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
common/multipath-over-rdma | 17 +++++------------
common/null_blk | 14 ++++++++++++++
tests/block/029 | 16 ++--------------
3 files changed, 21 insertions(+), 26 deletions(-)
diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index a56e7a8269db..7e610a0ccbbb 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -620,18 +620,11 @@ run_fio() {
configure_null_blk() {
local i
- (
- cd /sys/kernel/config/nullb || return $?
- for i in nullb0 nullb1; do (
- { mkdir -p $i &&
- cd $i &&
- echo 0 > completion_nsec &&
- echo 512 > blocksize &&
- echo $((ramdisk_size>>20)) > size &&
- echo 1 > memory_backed &&
- echo 1 > power; } || exit $?
- ) done
- )
+ for i in nullb0 nullb1; do
+ _configure_null_blk nullb$i completion_nsec=0 blocksize=512 \
+ size=$((ramdisk_size>>20)) memory_backed=1 \
+ power=1 || return $?
+ done
ls -l /dev/nullb* &>>"$FULL"
}
diff --git a/common/null_blk b/common/null_blk
index a4140e365955..08008fca8eae 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -29,6 +29,20 @@ _init_null_blk() {
return 0
}
+# Configure one null_blk instance with name $1 and parameters $2..${$#}.
+_configure_null_blk() {
+ local nullb=/sys/kernel/config/nullb/$1 param val
+
+ shift
+ mkdir "$nullb" || return $?
+ while [ -n "$1" ]; do
+ param="${1%%=*}"
+ val="${1#*=}"
+ shift
+ echo "$val" > "$nullb/$param" || return $?
+ done
+}
+
_exit_null_blk() {
_remove_null_blk_devices
udevadm settle
diff --git a/tests/block/029 b/tests/block/029
index 0d521edb0cf6..dbb582eab473 100755
--- a/tests/block/029
+++ b/tests/block/029
@@ -14,19 +14,6 @@ requires() {
_have_null_blk
}
-# Configure one null_blk instance.
-configure_null_blk() {
- local nullb0="/sys/kernel/config/nullb/nullb0"
-
- mkdir "$nullb0" &&
- echo 0 > "$nullb0/completion_nsec" &&
- echo 512 > "$nullb0/blocksize" &&
- echo 16 > "$nullb0/size" &&
- echo 1 > "$nullb0/memory_backed" &&
- echo 1 > "$nullb0/power" &&
- ls -l /dev/nullb* &>>"$FULL"
-}
-
modify_nr_hw_queues() {
local deadline num_cpus
@@ -45,7 +32,8 @@ test() {
: "${TIMEOUT:=30}"
_init_null_blk nr_devices=0 queue_mode=2 &&
- configure_null_blk
+ _configure_null_blk nullb0 completion_nsec=0 blocksize=512 \
+ size=16 memory_backed=1 power=1 &&
if { echo 1 >$sq; } 2>/dev/null; then
modify_nr_hw_queues &
fio --rw=randwrite --bs=4K --loops=$((10**6)) \
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-20 22:24 [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
` (2 preceding siblings ...)
2020-03-20 22:24 ` [PATCH blktests v3 3/4] Introduce the function _configure_null_blk() Bart Van Assche
@ 2020-03-20 22:24 ` Bart Van Assche
2020-03-23 11:29 ` Daniel Wagner
3 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-20 22:24 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-block, Chaitanya Kulkarni, Bart Van Assche, Ming Lei
Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
test only runs if a recently added fault injection feature is available,
namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
injection").
Cc: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
tests/block/030 | 42 ++++++++++++++++++++++++++++++++++++++++++
tests/block/030.out | 1 +
2 files changed, 43 insertions(+)
create mode 100755 tests/block/030
create mode 100644 tests/block/030.out
diff --git a/tests/block/030 b/tests/block/030
new file mode 100755
index 000000000000..4a17550ab8eb
--- /dev/null
+++ b/tests/block/030
@@ -0,0 +1,42 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2020 Google LLC
+#
+# Trigger the blk_mq_realloc_hw_ctxs() error path.
+
+. tests/block/rc
+. common/null_blk
+
+DESCRIPTION="trigger the blk_mq_realloc_hw_ctxs() error path"
+QUICK=1
+
+requires() {
+ _have_null_blk || return $?
+ _have_module_param null_blk init_hctx || return $?
+}
+
+test() {
+ local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
+
+ : "${TIMEOUT:=30}"
+ if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
+ echo "Loading null_blk failed"
+ return 1
+ fi
+ if ! _configure_null_blk nullb0 completion_nsec=0 blocksize=512 size=16\
+ submit_queues="$(nproc)" memory_backed=1 power=1; then
+ echo "Configuring null_blk failed"
+ return 1
+ fi
+ if { echo "$(<"$sq")" >$sq; } 2>/dev/null; then
+ for ((i=0;i<100;i++)); do
+ echo 1 >$sq
+ nproc >$sq
+ done
+ else
+ SKIP_REASON="Skipping test because $sq cannot be modified"
+ fi
+ rmdir /sys/kernel/config/nullb/nullb0
+ _exit_null_blk
+ echo Passed
+}
diff --git a/tests/block/030.out b/tests/block/030.out
new file mode 100644
index 000000000000..863339fb8ced
--- /dev/null
+++ b/tests/block/030.out
@@ -0,0 +1 @@
+Passed
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances
2020-03-20 22:24 ` [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances Bart Van Assche
@ 2020-03-23 11:09 ` Daniel Wagner
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Wagner @ 2020-03-23 11:09 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni
On Fri, Mar 20, 2020 at 03:24:10PM -0700, Bart Van Assche wrote:
> Instead of making every test remove null_blk device instances before calling
> _exit_null_blk(), move the null_blk device instance removal code into
> _exit_null_blk().
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions
2020-03-20 22:24 ` [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions Bart Van Assche
@ 2020-03-23 11:13 ` Daniel Wagner
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Wagner @ 2020-03-23 11:13 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni
On Fri, Mar 20, 2020 at 03:24:11PM -0700, Bart Van Assche wrote:
> This patch reduces code duplication.
>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 3/4] Introduce the function _configure_null_blk()
2020-03-20 22:24 ` [PATCH blktests v3 3/4] Introduce the function _configure_null_blk() Bart Van Assche
@ 2020-03-23 11:19 ` Daniel Wagner
2020-03-24 2:58 ` Bart Van Assche
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Wagner @ 2020-03-23 11:19 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni
Hi Bart,
On Fri, Mar 20, 2020 at 03:24:12PM -0700, Bart Van Assche wrote:
> Introduce a function for creating a null_blk device instance through
> configfs.
>
> Suggested-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> common/multipath-over-rdma | 17 +++++------------
> common/null_blk | 14 ++++++++++++++
> tests/block/029 | 16 ++--------------
> 3 files changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
> index a56e7a8269db..7e610a0ccbbb 100644
> --- a/common/multipath-over-rdma
> +++ b/common/multipath-over-rdma
> @@ -620,18 +620,11 @@ run_fio() {
> configure_null_blk() {
> local i
>
> - (
> - cd /sys/kernel/config/nullb || return $?
> - for i in nullb0 nullb1; do (
> - { mkdir -p $i &&
> - cd $i &&
> - echo 0 > completion_nsec &&
> - echo 512 > blocksize &&
> - echo $((ramdisk_size>>20)) > size &&
> - echo 1 > memory_backed &&
> - echo 1 > power; } || exit $?
> - ) done
> - )
> + for i in nullb0 nullb1; do
> + _configure_null_blk nullb$i completion_nsec=0 blocksize=512 \
I think this should be:
_configure_null_blk $i completion_nsec=0 blocksize=512 \
Rest looks good, feel free do add my Reviewed-by when spinning a new version.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-20 22:24 ` [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
@ 2020-03-23 11:29 ` Daniel Wagner
2020-03-24 3:09 ` Bart Van Assche
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Wagner @ 2020-03-23 11:29 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni, Ming Lei
Hi Bart,
On Fri, Mar 20, 2020 at 03:24:13PM -0700, Bart Van Assche wrote:
> Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
> Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
> test only runs if a recently added fault injection feature is available,
> namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
> injection").
[...]
> +test() {
> + local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
> +
> + : "${TIMEOUT:=30}"
> + if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
Doesn't make the $(nproc) the test subtil depending on the execution
environment?
Thanks,
Daniel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 3/4] Introduce the function _configure_null_blk()
2020-03-23 11:19 ` Daniel Wagner
@ 2020-03-24 2:58 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2020-03-24 2:58 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni
On 2020-03-23 04:19, Daniel Wagner wrote:
> On Fri, Mar 20, 2020 at 03:24:12PM -0700, Bart Van Assche wrote:
>> diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
>> index a56e7a8269db..7e610a0ccbbb 100644
>> --- a/common/multipath-over-rdma
>> +++ b/common/multipath-over-rdma
>> @@ -620,18 +620,11 @@ run_fio() {
>> configure_null_blk() {
>> local i
>>
>> - (
>> - cd /sys/kernel/config/nullb || return $?
>> - for i in nullb0 nullb1; do (
>> - { mkdir -p $i &&
>> - cd $i &&
>> - echo 0 > completion_nsec &&
>> - echo 512 > blocksize &&
>> - echo $((ramdisk_size>>20)) > size &&
>> - echo 1 > memory_backed &&
>> - echo 1 > power; } || exit $?
>> - ) done
>> - )
>> + for i in nullb0 nullb1; do
>> + _configure_null_blk nullb$i completion_nsec=0 blocksize=512 \
>
> I think this should be:
>
> _configure_null_blk $i completion_nsec=0 blocksize=512 \
Good catch, will fix.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-23 11:29 ` Daniel Wagner
@ 2020-03-24 3:09 ` Bart Van Assche
2020-03-24 10:41 ` Daniel Wagner
0 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-24 3:09 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni, Ming Lei
On 2020-03-23 04:29, Daniel Wagner wrote:
> On Fri, Mar 20, 2020 at 03:24:13PM -0700, Bart Van Assche wrote:
>> Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
>> Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
>> test only runs if a recently added fault injection feature is available,
>> namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
>> injection").
>
> [...]
>
>> +test() {
>> + local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
>> +
>> + : "${TIMEOUT:=30}"
>> + if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
>
> Doesn't make the $(nproc) the test subtil depending on the execution
> environment?
Hi Daniel,
The value $(nproc) has been chosen on purpose. The following code from
the test script:
+ echo 1 >$sq
+ nproc >$sq
triggers (nproc + 1) calls to null_init_hctx(). So injecting a failure
after (nproc) null_init_hctx() calls triggers the following pattern:
* The first blk_mq_realloc_hw_ctxs() call fails after (nproc - 1)
null_init_hctx() calls.
* The second blk_mq_realloc_hw_ctxs() call fails after (nproc - 2)
null_init_hctx() calls.
...
* The (nproc) th blk_mq_realloc_hw_ctxs() call fails after one
null_init_hctx() call.
* The (nproc + 1) th blk_mq_realloc_hw_ctxs() call succeeds.
I'm not sure to trigger this behavior without using the $(nproc) value?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-24 3:09 ` Bart Van Assche
@ 2020-03-24 10:41 ` Daniel Wagner
2020-03-25 2:10 ` Bart Van Assche
0 siblings, 1 reply; 14+ messages in thread
From: Daniel Wagner @ 2020-03-24 10:41 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni, Ming Lei
Hi Bart,
On Mon, Mar 23, 2020 at 08:09:27PM -0700, Bart Van Assche wrote:
> On 2020-03-23 04:29, Daniel Wagner wrote:
> > On Fri, Mar 20, 2020 at 03:24:13PM -0700, Bart Van Assche wrote:
> >> +test() {
> >> + local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
> >> +
> >> + : "${TIMEOUT:=30}"
> >> + if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
From the kernel code:
/* "<interval>,<probability>,<space>,<times>" */
Don't you need to set the times attribute to -1 in order to inject the
everytime the interval is reached? If I understood it correctly,
with times=0 no failure is injected.
BTW, I've had to change it to init_hctx=$(($(nproc)+1)) to pass
the initial __configure_null_blk call before the first fail hits.
> > Doesn't make the $(nproc) the test subtil depending on the execution
> > environment?
> The value $(nproc) has been chosen on purpose. The following code from
> the test script:
>
> + echo 1 >$sq
> + nproc >$sq
>
> triggers (nproc + 1) calls to null_init_hctx().So injecting a failure
> after (nproc) null_init_hctx() calls triggers the following pattern:
> * The first blk_mq_realloc_hw_ctxs() call fails after (nproc - 1)
> null_init_hctx() calls.
> * The second blk_mq_realloc_hw_ctxs() call fails after (nproc - 2)
> null_init_hctx() calls.
> ...
> * The (nproc) th blk_mq_realloc_hw_ctxs() call fails after one
> null_init_hctx() call.
> * The (nproc + 1) th blk_mq_realloc_hw_ctxs() call succeeds.
>
> I'm not sure to trigger this behavior without using the $(nproc) value?
Okay, I get the idea how you want to test.
Is the dependency on nproc because null_blk expects submit_queue <= online
cpus?
Though why the 100?
for ((i=0;i<100;i++)); do
echo 1 >$sq
nproc >$sq
done
And shouldn't be there a test for error?
Thanks,
Daniel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-24 10:41 ` Daniel Wagner
@ 2020-03-25 2:10 ` Bart Van Assche
2020-03-25 8:18 ` Daniel Wagner
0 siblings, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2020-03-25 2:10 UTC (permalink / raw)
To: Daniel Wagner; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni, Ming Lei
On 2020-03-24 03:41, Daniel Wagner wrote:
> Hi Bart,
>
> On Mon, Mar 23, 2020 at 08:09:27PM -0700, Bart Van Assche wrote:
>> On 2020-03-23 04:29, Daniel Wagner wrote:
>>> On Fri, Mar 20, 2020 at 03:24:13PM -0700, Bart Van Assche wrote:
>>>> +test() {
>>>> + local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
>>>> +
>>>> + : "${TIMEOUT:=30}"
>>>> + if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
>
>>From the kernel code:
>
> /* "<interval>,<probability>,<space>,<times>" */
>
> Don't you need to set the times attribute to -1 in order to inject the
> everytime the interval is reached? If I understood it correctly,
> with times=0 no failure is injected.
>
> BTW, I've had to change it to init_hctx=$(($(nproc)+1)) to pass
> the initial __configure_null_blk call before the first fail hits.
Hi Daniel,
I will make both changes in the init_hctx string. Not sure how this
escaped from my attention.
>>> Doesn't make the $(nproc) the test subtil depending on the execution
>>> environment?
>> The value $(nproc) has been chosen on purpose. The following code from
>> the test script:
>>
>> + echo 1 >$sq
>> + nproc >$sq
>>
>> triggers (nproc + 1) calls to null_init_hctx().So injecting a failure
>> after (nproc) null_init_hctx() calls triggers the following pattern:
>> * The first blk_mq_realloc_hw_ctxs() call fails after (nproc - 1)
>> null_init_hctx() calls.
>> * The second blk_mq_realloc_hw_ctxs() call fails after (nproc - 2)
>> null_init_hctx() calls.
>> ...
>> * The (nproc) th blk_mq_realloc_hw_ctxs() call fails after one
>> null_init_hctx() call.
>> * The (nproc + 1) th blk_mq_realloc_hw_ctxs() call succeeds.
>>
>> I'm not sure to trigger this behavior without using the $(nproc) value?
>
> Okay, I get the idea how you want to test.
>
> Is the dependency on nproc because null_blk expects submit_queue <= online
> cpus?
That's correct. I want to test with the maximum number of submit queues
allowed, hence the use of $(nproc).
> Though why the 100?
>
> for ((i=0;i<100;i++)); do
> echo 1 >$sq
> nproc >$sq
> done
No particular reason other than "a significant number of iterations".
> And shouldn't be there a test for error?
All I want to test is the absence of kernel crashes. The blktests
framework already inspects dmesg for the absence of kernel crashes. So I
don't think that I have to check whether or not the quoted sysfs writes
succeed.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
2020-03-25 2:10 ` Bart Van Assche
@ 2020-03-25 8:18 ` Daniel Wagner
0 siblings, 0 replies; 14+ messages in thread
From: Daniel Wagner @ 2020-03-25 8:18 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Omar Sandoval, linux-block, Chaitanya Kulkarni, Ming Lei
Hi Bart,
> > Is the dependency on nproc because null_blk expects submit_queue <= online
> > cpus?
>
> That's correct. I want to test with the maximum number of submit queues
> allowed, hence the use of $(nproc).
Ok, I'd probably add this info as comment or have it in the commit
message. Just in case someone in the future wonders why nproc.
> > Though why the 100?
> >
> > for ((i=0;i<100;i++)); do
> > echo 1 >$sq
> > nproc >$sq
> > done
>
> No particular reason other than "a significant number of iterations".
>
> > And shouldn't be there a test for error?
Ah ok. I was getting paranoid about numbers :)
> All I want to test is the absence of kernel crashes. The blktests
> framework already inspects dmesg for the absence of kernel crashes. So I
> don't think that I have to check whether or not the quoted sysfs writes
> succeed.
Maybe adding this info to the commit message what this test tries to
cover wouldn't hurt.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-03-25 8:18 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 22:24 [PATCH blktests v3 0/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 1/4] Make _exit_null_blk remove all null_blk device instances Bart Van Assche
2020-03-23 11:09 ` Daniel Wagner
2020-03-20 22:24 ` [PATCH blktests v3 2/4] Use _{init,exit}_null_blk instead of open-coding these functions Bart Van Assche
2020-03-23 11:13 ` Daniel Wagner
2020-03-20 22:24 ` [PATCH blktests v3 3/4] Introduce the function _configure_null_blk() Bart Van Assche
2020-03-23 11:19 ` Daniel Wagner
2020-03-24 2:58 ` Bart Van Assche
2020-03-20 22:24 ` [PATCH blktests v3 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path Bart Van Assche
2020-03-23 11:29 ` Daniel Wagner
2020-03-24 3:09 ` Bart Van Assche
2020-03-24 10:41 ` Daniel Wagner
2020-03-25 2:10 ` Bart Van Assche
2020-03-25 8:18 ` Daniel Wagner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.