* [PATCH V2 0/2] *** Test the NVMe reservation feature ***
@ 2024-01-17 8:17 Guixin Liu
2024-01-17 8:17 ` [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument Guixin Liu
2024-01-17 8:17 ` [PATCH V2 2/2] test/nvme/050: test the reservation feature Guixin Liu
0 siblings, 2 replies; 7+ messages in thread
From: Guixin Liu @ 2024-01-17 8:17 UTC (permalink / raw)
To: shinichiro.kawasaki; +Cc: chaitanyak, linux-block, linux-nvme
Changes from v1 to v2:
- Add a new patch to add a optinal argument --resv_enable.
- Make resv-report fit the new --eds argument.
- Filter the hosid output to make the new test case independent from
hostid.
- Change the new test case name to "Test the NVMe reservation feature".
Guixin Liu (2):
nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument
test/nvme/050: test the reservation feature
tests/nvme/002 | 3 +-
tests/nvme/016 | 7 ++-
tests/nvme/017 | 10 +++--
tests/nvme/030 | 6 ++-
tests/nvme/031 | 3 +-
tests/nvme/050 | 96 ++++++++++++++++++++++++++++++++++++++++
tests/nvme/050.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
tests/nvme/rc | 100 ++++++++++++++++++++++++++++++++++-------
8 files changed, 308 insertions(+), 25 deletions(-)
create mode 100644 tests/nvme/050
create mode 100644 tests/nvme/050.out
--
2.30.1 (Apple Git-130)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument
2024-01-17 8:17 [PATCH V2 0/2] *** Test the NVMe reservation feature *** Guixin Liu
@ 2024-01-17 8:17 ` Guixin Liu
2024-01-23 10:54 ` Shinichiro Kawasaki
2024-01-17 8:17 ` [PATCH V2 2/2] test/nvme/050: test the reservation feature Guixin Liu
1 sibling, 1 reply; 7+ messages in thread
From: Guixin Liu @ 2024-01-17 8:17 UTC (permalink / raw)
To: shinichiro.kawasaki; +Cc: chaitanyak, linux-block, linux-nvme
Add an optional argument --resv_enable to _nvmet_target_setup() and
propagate it to _create_nvmet_subsystem() and _create_nvmet_ns().
One can call functions with --resv_enable to enable reservation
feature on a specific namespace.
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
tests/nvme/002 | 3 +-
tests/nvme/016 | 7 +++-
tests/nvme/017 | 10 +++--
tests/nvme/030 | 6 ++-
tests/nvme/031 | 3 +-
tests/nvme/rc | 100 +++++++++++++++++++++++++++++++++++++++++--------
6 files changed, 104 insertions(+), 25 deletions(-)
diff --git a/tests/nvme/002 b/tests/nvme/002
index 6b84848..37c719b 100755
--- a/tests/nvme/002
+++ b/tests/nvme/002
@@ -30,7 +30,8 @@ test() {
local genctr=1
for ((i = 0; i < iterations; i++)); do
- _create_nvmet_subsystem "blktests-subsystem-$i" "${loop_dev}"
+ _create_nvmet_subsystem --subsysnqn "blktests-subsystem-$i" \
+ --blkdev "${loop_dev}"
_add_nvmet_subsys_to_port "${port}" "blktests-subsystem-$i"
done
diff --git a/tests/nvme/016 b/tests/nvme/016
index 908abbd..ba700d1 100755
--- a/tests/nvme/016
+++ b/tests/nvme/016
@@ -25,10 +25,13 @@ test() {
loop_dev="$(losetup -f)"
local genctr=1
- _create_nvmet_subsystem "${def_subsysnqn}" "${loop_dev}"
+ _create_nvmet_subsystem --subsysnqn "${def_subsysnqn}" \
+ --blkdev "${loop_dev}"
for ((i = 2; i <= iterations; i++)); do
- _create_nvmet_ns "${def_subsysnqn}" "${i}" "${loop_dev}"
+ _create_nvmet_ns --subsysnqn "${def_subsysnqn}" \
+ --nsid "${i}" \
+ --blkdev "${loop_dev}"
done
port="$(_create_nvmet_port "${nvme_trtype}")"
diff --git a/tests/nvme/017 b/tests/nvme/017
index c8d9b32..d9fbd38 100755
--- a/tests/nvme/017
+++ b/tests/nvme/017
@@ -25,12 +25,14 @@ test() {
local genctr=1
- _create_nvmet_subsystem "${def_subsysnqn}" "$(_nvme_def_file_path)" \
- "${def_subsys_uuid}"
+ _create_nvmet_subsystem --subsysnqn "${def_subsysnqn}" \
+ --blkdev "$(_nvme_def_file_path)" \
+ --uuid "${def_subsys_uuid}"
for ((i = 2; i <= iterations; i++)); do
- _create_nvmet_ns "${def_subsysnqn}" "${i}" \
- "$(_nvme_def_file_path)"
+ _create_nvmet_ns --subsysnqn "${def_subsysnqn}" \
+ --nsid "${i}" \
+ --blkdev "$(_nvme_def_file_path)"
done
port="$(_create_nvmet_port "${nvme_trtype}")"
diff --git a/tests/nvme/030 b/tests/nvme/030
index 9251e17..8802b16 100755
--- a/tests/nvme/030
+++ b/tests/nvme/030
@@ -26,13 +26,15 @@ test() {
port="$(_create_nvmet_port "${nvme_trtype}")"
- _create_nvmet_subsystem "${subsys}1" "$(losetup -f)"
+ _create_nvmet_subsystem --subsysnqn "${subsys}1" \
+ --blkdev "$(losetup -f)"
_add_nvmet_subsys_to_port "${port}" "${subsys}1"
_create_nvmet_host "${subsys}1" "${def_hostnqn}"
genctr=$(_discovery_genctr)
- _create_nvmet_subsystem "${subsys}2" "$(losetup -f)"
+ _create_nvmet_subsystem --subsysnqn "${subsys}2" \
+ --blkdev "$(losetup -f)"
_add_nvmet_subsys_to_port "${port}" "${subsys}2"
genctr=$(_check_genctr "${genctr}" "adding a subsystem to a port")
diff --git a/tests/nvme/031 b/tests/nvme/031
index ed5f196..191ce72 100755
--- a/tests/nvme/031
+++ b/tests/nvme/031
@@ -40,7 +40,8 @@ test() {
port="$(_create_nvmet_port "${nvme_trtype}")"
for ((i = 0; i < iterations; i++)); do
- _create_nvmet_subsystem "${subsys}$i" "${loop_dev}"
+ _create_nvmet_subsystem --subsysnqn "${subsys}$i" \
+ --blkdev "${loop_dev}"
_add_nvmet_subsys_to_port "${port}" "${subsys}$i"
_create_nvmet_host "${subsys}$i" "${def_hostnqn}"
_nvme_connect_subsys "${nvme_trtype}" "${subsys}$i"
diff --git a/tests/nvme/rc b/tests/nvme/rc
index b0ef1ee..c6466cc 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -656,32 +656,95 @@ _remove_nvmet_port() {
}
_create_nvmet_ns() {
- local nvmet_subsystem="$1"
- local nsid="$2"
- local blkdev="$3"
+ local nvmet_subsystem=""
+ local nsid=""
+ local blkdev=""
local uuid="00000000-0000-0000-0000-000000000000"
- local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
- local ns_path="${subsys_path}/namespaces/${nsid}"
+ local subsys_path=""
+ local ns_path=""
+ local resv_enable=false
- if [[ $# -eq 4 ]]; then
- uuid="$4"
- fi
+ while [[ $# -gt 0 ]]; do
+ case $1 in
+ --subsysnqn)
+ nvmet_subsystem="$2"
+ shift 2
+ ;;
+ --nsid)
+ nsid="$2"
+ shift 2
+ ;;
+ --blkdev)
+ blkdev="$2"
+ shift 2
+ ;;
+ --uuid)
+ uuid="$2"
+ shift 2
+ ;;
+ --resv_enable)
+ resv_enable=true
+ shift 1
+ ;;
+ *)
+ echo "WARNING: unknown argument: $1"
+ shift
+ ;;
+ esac
+ done
+
+ subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+ ns_path="${subsys_path}/namespaces/${nsid}"
mkdir "${ns_path}"
printf "%s" "${blkdev}" > "${ns_path}/device_path"
printf "%s" "${uuid}" > "${ns_path}/device_uuid"
+ if [[ -f "${ns_path}/resv_enable" && "${resv_enable}" = true ]] ; then
+ printf 1 > "${ns_path}/resv_enable"
+ fi
printf 1 > "${ns_path}/enable"
}
_create_nvmet_subsystem() {
- local nvmet_subsystem="$1"
- local blkdev="$2"
- local uuid=$3
- local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+ local nvmet_subsystem=""
+ local blkdev=""
+ local uuid="00000000-0000-0000-0000-000000000000"
+ local resv_enable=""
+ local cfs_path=""
+ while [[ $# -gt 0 ]]; do
+ case $1 in
+ --subsysnqn)
+ nvmet_subsystem="$2"
+ shift 2
+ ;;
+ --blkdev)
+ blkdev="$2"
+ shift 2
+ ;;
+ --uuid)
+ uuid="$2"
+ shift 2
+ ;;
+ --resv_enable)
+ resv_enable="--resv_enable";
+ shift 1
+ ;;
+ *)
+ echo "WARNING: unknown argument: $1"
+ shift
+ ;;
+ esac
+ done
+
+ cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
mkdir -p "${cfs_path}"
echo 0 > "${cfs_path}/attr_allow_any_host"
- _create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
+ _create_nvmet_ns --subsysnqn "${nvmet_subsystem}" \
+ --nsid "1" \
+ --blkdev "${blkdev}" \
+ --uuid "${uuid}" \
+ ${resv_enable}
}
_add_nvmet_allow_hosts() {
@@ -863,6 +926,7 @@ _nvmet_target_setup() {
local ctrlkey=""
local hostkey=""
local port
+ local resv_enable=""
while [[ $# -gt 0 ]]; do
case $1 in
@@ -878,6 +942,10 @@ _nvmet_target_setup() {
hostkey="$2"
shift 2
;;
+ --resv_enable)
+ resv_enable="--resv_enable"
+ shift 1
+ ;;
*)
echo "WARNING: unknown argument: $1"
shift
@@ -892,8 +960,10 @@ _nvmet_target_setup() {
blkdev="$(_nvme_def_file_path)"
fi
- _create_nvmet_subsystem "${def_subsysnqn}" "${blkdev}" \
- "${def_subsys_uuid}"
+ _create_nvmet_subsystem --subsysnqn "${def_subsysnqn}" \
+ --blkdev "${blkdev}" \
+ --uuid "${def_subsys_uuid}" \
+ ${resv_enable}
port="$(_create_nvmet_port "${nvme_trtype}")"
_add_nvmet_subsys_to_port "${port}" "${def_subsysnqn}"
_create_nvmet_host "${def_subsysnqn}" "${def_hostnqn}" \
--
2.30.1 (Apple Git-130)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V2 2/2] test/nvme/050: test the reservation feature
2024-01-17 8:17 [PATCH V2 0/2] *** Test the NVMe reservation feature *** Guixin Liu
2024-01-17 8:17 ` [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument Guixin Liu
@ 2024-01-17 8:17 ` Guixin Liu
2024-01-23 11:21 ` Shinichiro Kawasaki
1 sibling, 1 reply; 7+ messages in thread
From: Guixin Liu @ 2024-01-17 8:17 UTC (permalink / raw)
To: shinichiro.kawasaki; +Cc: chaitanyak, linux-block, linux-nvme
Test the reservation feature, includes register, acquire, release
and report.
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
tests/nvme/050 | 96 ++++++++++++++++++++++++++++++++++++++++
tests/nvme/050.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 204 insertions(+)
create mode 100644 tests/nvme/050
create mode 100644 tests/nvme/050.out
diff --git a/tests/nvme/050 b/tests/nvme/050
new file mode 100644
index 0000000..7e59de4
--- /dev/null
+++ b/tests/nvme/050
@@ -0,0 +1,96 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Guixin Liu
+# Copyright (C) 2024 Alibaba Group.
+#
+# Test the NVMe reservation feature
+#
+. tests/nvme/rc
+
+DESCRIPTION="test the reservation feature"
+QUICK=1
+
+requires() {
+ _nvme_requires
+}
+
+resv_report() {
+ local nvmedev=$1
+
+ if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
+ nvme resv-report "/dev/${nvmedev}n1" --eds | grep -v "hostid"
+ else
+ nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 | grep -v "hostid"
+ fi
+}
+
+test_resv() {
+ local nvmedev=$1
+
+ echo "Register"
+ resv_report "${nvmedev}"
+ nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+ resv_report "${nvmedev}"
+
+ echo "Replace"
+ nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2
+ resv_report "${nvmedev}"
+
+ echo "Unregister"
+ nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1
+ resv_report "${nvmedev}"
+
+ echo "Acquire"
+ nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+ nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
+ resv_report "${nvmedev}"
+
+ echo "Preempt"
+ nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1
+ resv_report "${nvmedev}"
+
+ echo "Release"
+ nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0
+ resv_report "${nvmedev}"
+
+ echo "Clear"
+ nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0
+ nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0
+ resv_report "${nvmedev}"
+ nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1
+}
+
+test() {
+ echo "Running ${TEST_NAME}"
+
+ _setup_nvmet
+
+ local nvmedev
+ local skipped=false
+ local subsys_path=""
+ local ns_path=""
+
+ _nvmet_target_setup --blkdev file --resv_enable
+ subsys_path="${NVMET_CFS}/subsystems/${def_subsysnqn}"
+ ns_path="${subsys_path}/namespaces/1"
+
+ if [[ -f "${ns_path}/resv_enable" ]] ; then
+ _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}"
+
+ nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
+
+ test_resv "${nvmedev}"
+ _nvme_disconnect_subsys "${def_subsysnqn}"
+ else
+ SKIP_REASONS+=("missing reservation feature")
+ skipped=true
+ fi
+
+ _nvmet_target_cleanup
+
+ if [[ "${skipped}" = true ]] ; then
+ return 1
+ fi
+
+ echo "Test complete"
+}
diff --git a/tests/nvme/050.out b/tests/nvme/050.out
new file mode 100644
index 0000000..2a46b32
--- /dev/null
+++ b/tests/nvme/050.out
@@ -0,0 +1,108 @@
+Running nvme/050
+Register
+
+NVME Reservation status:
+
+gen : 0
+rtype : 0
+regctl : 0
+ptpls : 0
+
+NVME Reservation success
+
+NVME Reservation status:
+
+gen : 1
+rtype : 0
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 0
+ rkey : 4
+
+Replace
+NVME Reservation success
+
+NVME Reservation status:
+
+gen : 2
+rtype : 0
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 0
+ rkey : 5
+
+Unregister
+NVME Reservation success
+
+NVME Reservation status:
+
+gen : 3
+rtype : 0
+regctl : 0
+ptpls : 0
+
+Acquire
+NVME Reservation success
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen : 4
+rtype : 1
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 1
+ rkey : 4
+
+Preempt
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen : 5
+rtype : 2
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 1
+ rkey : 4
+
+Release
+NVME Reservation Release success
+
+NVME Reservation status:
+
+gen : 5
+rtype : 0
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 0
+ rkey : 4
+
+Clear
+NVME Reservation success
+NVME Reservation Acquire success
+
+NVME Reservation status:
+
+gen : 6
+rtype : 1
+regctl : 1
+ptpls : 0
+regctlext[0] :
+ cntlid : 1
+ rcsts : 1
+ rkey : 4
+
+NVME Reservation Release success
+disconnected 1 controller(s)
+Test complete
--
2.30.1 (Apple Git-130)
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument
2024-01-17 8:17 ` [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument Guixin Liu
@ 2024-01-23 10:54 ` Shinichiro Kawasaki
2024-01-31 14:47 ` Daniel Wagner
0 siblings, 1 reply; 7+ messages in thread
From: Shinichiro Kawasaki @ 2024-01-23 10:54 UTC (permalink / raw)
To: Guixin Liu
Cc: chaitanyak@nvidia.com, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org, dwagner@suse.de
On Jan 17, 2024 / 16:17, Guixin Liu wrote:
> Add an optional argument --resv_enable to _nvmet_target_setup() and
> propagate it to _create_nvmet_subsystem() and _create_nvmet_ns().
>
> One can call functions with --resv_enable to enable reservation
> feature on a specific namespace.
>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Thanks, looks good to me.
Daniel, could you take a look in this patch? I think it is consistent with your
work on _nvme_connect_subsys() and _nvmet_target_setup() in the past.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 2/2] test/nvme/050: test the reservation feature
2024-01-17 8:17 ` [PATCH V2 2/2] test/nvme/050: test the reservation feature Guixin Liu
@ 2024-01-23 11:21 ` Shinichiro Kawasaki
2024-01-23 11:41 ` Guixin Liu
0 siblings, 1 reply; 7+ messages in thread
From: Shinichiro Kawasaki @ 2024-01-23 11:21 UTC (permalink / raw)
To: Guixin Liu
Cc: chaitanyak@nvidia.com, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org
On Jan 17, 2024 / 16:17, Guixin Liu wrote:
> Test the reservation feature, includes register, acquire, release
> and report.
>
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Thanks for this v2. I ran it with kernel side v4 patch [1], enabling lockdep.
And I observed lockdep WARN [2]. For your reference, I attached the WARN at
the end of this e-mail.
[1] https://lore.kernel.org/linux-nvme/20240118125057.56200-2-kanie@linux.alibaba.com/
This blktests patch looks almost good for me. Please find minor nit comments
in line.
> ---
> tests/nvme/050 | 96 ++++++++++++++++++++++++++++++++++++++++
> tests/nvme/050.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 204 insertions(+)
> create mode 100644 tests/nvme/050
> create mode 100644 tests/nvme/050.out
>
> diff --git a/tests/nvme/050 b/tests/nvme/050
> new file mode 100644
> index 0000000..7e59de4
> --- /dev/null
> +++ b/tests/nvme/050
> @@ -0,0 +1,96 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Guixin Liu
> +# Copyright (C) 2024 Alibaba Group.
> +#
> +# Test the NVMe reservation feature
> +#
> +. tests/nvme/rc
> +
> +DESCRIPTION="test the reservation feature"
> +QUICK=1
> +
> +requires() {
> + _nvme_requires
> +}
> +
> +resv_report() {
> + local nvmedev=$1
> +
> + if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
It feels costly to call "resv-report --help" multiple times. I suggest to call
it only once at the beginning of test_resv(). Based on the check result, a local
variable can be set up and passed to resv_report().
> + nvme resv-report "/dev/${nvmedev}n1" --eds | grep -v "hostid"
> + else
> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 | grep -v "hostid"
The two lines above are almost same. I think they can be unified with the
variable passed from the caller.
> + fi
> +}
> +
[...]
[2]
run blktests nvme/050 at 2024-01-23 19:05:08
nvmet: adding nsid 1 to subsystem blktests-subsystem-1
nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349.
nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.
nvme nvme1: creating 4 I/O queues.
nvme nvme1: new ctrl: "blktests-subsystem-1"
nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1"
======================================================
WARNING: possible circular locking dependency detected
6.7.0+ #142 Not tainted
------------------------------------------------------
check/1061 is trying to acquire lock:
ffff888139743a78 (&ns->pr.pr_lock){+.+.}-{3:3}, at: nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
but task is already holding lock:
ffff888110cf7070 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_ns_disable+0x2a2/0x4a0 [nvmet]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&subsys->lock#2){+.+.}-{3:3}:
__mutex_lock+0x185/0x18c0
nvmet_pr_send_resv_released+0x57/0x220 [nvmet]
nvmet_pr_preempt+0x651/0xc80 [nvmet]
nvmet_execute_pr_acquire+0x26f/0x5c0 [nvmet]
process_one_work+0x74c/0x1260
worker_thread+0x723/0x1300
kthread+0x2f1/0x3d0
ret_from_fork+0x30/0x70
ret_from_fork_asm+0x1b/0x30
-> #0 (&ns->pr.pr_lock){+.+.}-{3:3}:
__lock_acquire+0x2e96/0x5f40
lock_acquire+0x1a9/0x4e0
__mutex_lock+0x185/0x18c0
nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
nvmet_ns_disable+0x313/0x4a0 [nvmet]
nvmet_ns_enable_store+0x8a/0xe0 [nvmet]
configfs_write_iter+0x2ae/0x460
vfs_write+0x540/0xd90
ksys_write+0xf7/0x1d0
do_syscall_64+0x60/0xe0
entry_SYSCALL_64_after_hwframe+0x6e/0x76
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&subsys->lock#2);
lock(&ns->pr.pr_lock);
lock(&subsys->lock#2);
lock(&ns->pr.pr_lock);
*** DEADLOCK ***
4 locks held by check/1061:
#0: ffff88813a8e8418 (sb_writers#14){.+.+}-{0:0}, at: ksys_write+0xf7/0x1d0
#1: ffff88811e893a88 (&buffer->mutex){+.+.}-{3:3}, at: configfs_write_iter+0x73/0x460
#2: ffff88812e673978 (&p->frag_sem){++++}-{3:3}, at: configfs_write_iter+0x1db/0x460
#3: ffff888110cf7070 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_ns_disable+0x2a2/0x4a0 [nvmet]
stack backtrace:
CPU: 0 PID: 1061 Comm: check Not tainted 6.7.0+ #142
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
Call Trace:
<TASK>
dump_stack_lvl+0x57/0x90
check_noncircular+0x309/0x3f0
? __pfx_check_noncircular+0x10/0x10
? lockdep_lock+0xca/0x1c0
? __pfx_lockdep_lock+0x10/0x10
? lock_release+0x378/0x650
? __stack_depot_save+0x246/0x470
__lock_acquire+0x2e96/0x5f40
? __pfx___lock_acquire+0x10/0x10
lock_acquire+0x1a9/0x4e0
? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
? __pfx_lock_acquire+0x10/0x10
? lock_is_held_type+0xce/0x120
? __pfx_lock_acquire+0x10/0x10
? __pfx___might_resched+0x10/0x10
__mutex_lock+0x185/0x18c0
? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
? rcu_is_watching+0x11/0xb0
? __mutex_lock+0x2a2/0x18c0
? __pfx___mutex_lock+0x10/0x10
? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
nvmet_ns_disable+0x313/0x4a0 [nvmet]
? __pfx_nvmet_ns_disable+0x10/0x10 [nvmet]
nvmet_ns_enable_store+0x8a/0xe0 [nvmet]
? __pfx_nvmet_ns_enable_store+0x10/0x10 [nvmet]
configfs_write_iter+0x2ae/0x460
vfs_write+0x540/0xd90
? __pfx_vfs_write+0x10/0x10
? __pfx___lock_acquire+0x10/0x10
? __handle_mm_fault+0x12c5/0x1870
? __fget_light+0x51/0x220
ksys_write+0xf7/0x1d0
? __pfx_ksys_write+0x10/0x10
? syscall_enter_from_user_mode+0x22/0x90
do_syscall_64+0x60/0xe0
? __pfx_lock_release+0x10/0x10
? count_memcg_events.constprop.0+0x4a/0x60
? handle_mm_fault+0x1b1/0x9d0
? exc_page_fault+0xc0/0x100
? rcu_is_watching+0x11/0xb0
? asm_exc_page_fault+0x22/0x30
? lockdep_hardirqs_on+0x7d/0x100
entry_SYSCALL_64_after_hwframe+0x6e/0x76
RIP: 0033:0x7f604525ac34
Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 35 77 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 55 48 89 e5 48 83 ec 20 48 89
RSP: 002b:00007ffec7fd6ce8 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f604525ac34
RDX: 0000000000000002 RSI: 0000562b0cd805a0 RDI: 0000000000000001
RBP: 00007ffec7fd6d10 R08: 0000000000001428 R09: 0000000100000000
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000002
R13: 0000562b0cd805a0 R14: 00007f604532b5c0 R15: 00007f6045328f20
</TASK>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2 2/2] test/nvme/050: test the reservation feature
2024-01-23 11:21 ` Shinichiro Kawasaki
@ 2024-01-23 11:41 ` Guixin Liu
0 siblings, 0 replies; 7+ messages in thread
From: Guixin Liu @ 2024-01-23 11:41 UTC (permalink / raw)
To: Shinichiro Kawasaki
Cc: chaitanyak@nvidia.com, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org
在 2024/1/23 19:21, Shinichiro Kawasaki 写道:
> On Jan 17, 2024 / 16:17, Guixin Liu wrote:
>> Test the reservation feature, includes register, acquire, release
>> and report.
>>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Thanks for this v2. I ran it with kernel side v4 patch [1], enabling lockdep.
> And I observed lockdep WARN [2]. For your reference, I attached the WARN at
> the end of this e-mail.
>
> [1] https://lore.kernel.org/linux-nvme/20240118125057.56200-2-kanie@linux.alibaba.com/
>
> This blktests patch looks almost good for me. Please find minor nit comments
> in line.
>
>> ---
>> tests/nvme/050 | 96 ++++++++++++++++++++++++++++++++++++++++
>> tests/nvme/050.out | 108 +++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 204 insertions(+)
>> create mode 100644 tests/nvme/050
>> create mode 100644 tests/nvme/050.out
>>
>> diff --git a/tests/nvme/050 b/tests/nvme/050
>> new file mode 100644
>> index 0000000..7e59de4
>> --- /dev/null
>> +++ b/tests/nvme/050
>> @@ -0,0 +1,96 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Guixin Liu
>> +# Copyright (C) 2024 Alibaba Group.
>> +#
>> +# Test the NVMe reservation feature
>> +#
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="test the reservation feature"
>> +QUICK=1
>> +
>> +requires() {
>> + _nvme_requires
>> +}
>> +
>> +resv_report() {
>> + local nvmedev=$1
>> +
>> + if nvme resv-report --help 2>&1 | grep -- '--eds' > /dev/null; then
> It feels costly to call "resv-report --help" multiple times. I suggest to call
> it only once at the beginning of test_resv(). Based on the check result, a local
> variable can be set up and passed to resv_report().
OK, I will change it in v3.
>> + nvme resv-report "/dev/${nvmedev}n1" --eds | grep -v "hostid"
>> + else
>> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 | grep -v "hostid"
> The two lines above are almost same. I think they can be unified with the
> variable passed from the caller.
OK, I will change it in v3.
>
>> + fi
>> +}
>> +
> [...]
>
> [2]
>
> run blktests nvme/050 at 2024-01-23 19:05:08
> nvmet: adding nsid 1 to subsystem blktests-subsystem-1
> nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349.
> nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices.
> nvme nvme1: creating 4 I/O queues.
> nvme nvme1: new ctrl: "blktests-subsystem-1"
> nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1"
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.7.0+ #142 Not tainted
> ------------------------------------------------------
> check/1061 is trying to acquire lock:
> ffff888139743a78 (&ns->pr.pr_lock){+.+.}-{3:3}, at: nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
>
> but task is already holding lock:
> ffff888110cf7070 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_ns_disable+0x2a2/0x4a0 [nvmet]
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (&subsys->lock#2){+.+.}-{3:3}:
> __mutex_lock+0x185/0x18c0
> nvmet_pr_send_resv_released+0x57/0x220 [nvmet]
> nvmet_pr_preempt+0x651/0xc80 [nvmet]
> nvmet_execute_pr_acquire+0x26f/0x5c0 [nvmet]
> process_one_work+0x74c/0x1260
> worker_thread+0x723/0x1300
> kthread+0x2f1/0x3d0
> ret_from_fork+0x30/0x70
> ret_from_fork_asm+0x1b/0x30
>
> -> #0 (&ns->pr.pr_lock){+.+.}-{3:3}:
> __lock_acquire+0x2e96/0x5f40
> lock_acquire+0x1a9/0x4e0
> __mutex_lock+0x185/0x18c0
> nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> nvmet_ns_disable+0x313/0x4a0 [nvmet]
> nvmet_ns_enable_store+0x8a/0xe0 [nvmet]
> configfs_write_iter+0x2ae/0x460
> vfs_write+0x540/0xd90
> ksys_write+0xf7/0x1d0
> do_syscall_64+0x60/0xe0
> entry_SYSCALL_64_after_hwframe+0x6e/0x76
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(&subsys->lock#2);
> lock(&ns->pr.pr_lock);
> lock(&subsys->lock#2);
> lock(&ns->pr.pr_lock);
>
> *** DEADLOCK ***
>
> 4 locks held by check/1061:
> #0: ffff88813a8e8418 (sb_writers#14){.+.+}-{0:0}, at: ksys_write+0xf7/0x1d0
> #1: ffff88811e893a88 (&buffer->mutex){+.+.}-{3:3}, at: configfs_write_iter+0x73/0x460
> #2: ffff88812e673978 (&p->frag_sem){++++}-{3:3}, at: configfs_write_iter+0x1db/0x460
> #3: ffff888110cf7070 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_ns_disable+0x2a2/0x4a0 [nvmet]
>
> stack backtrace:
> CPU: 0 PID: 1061 Comm: check Not tainted 6.7.0+ #142
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x57/0x90
> check_noncircular+0x309/0x3f0
> ? __pfx_check_noncircular+0x10/0x10
> ? lockdep_lock+0xca/0x1c0
> ? __pfx_lockdep_lock+0x10/0x10
> ? lock_release+0x378/0x650
> ? __stack_depot_save+0x246/0x470
> __lock_acquire+0x2e96/0x5f40
> ? __pfx___lock_acquire+0x10/0x10
> lock_acquire+0x1a9/0x4e0
> ? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> ? __pfx_lock_acquire+0x10/0x10
> ? lock_is_held_type+0xce/0x120
> ? __pfx_lock_acquire+0x10/0x10
> ? __pfx___might_resched+0x10/0x10
> __mutex_lock+0x185/0x18c0
> ? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> ? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> ? rcu_is_watching+0x11/0xb0
> ? __mutex_lock+0x2a2/0x18c0
> ? __pfx___mutex_lock+0x10/0x10
> ? nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> nvmet_pr_exit_ns+0x2e/0x230 [nvmet]
> nvmet_ns_disable+0x313/0x4a0 [nvmet]
> ? __pfx_nvmet_ns_disable+0x10/0x10 [nvmet]
> nvmet_ns_enable_store+0x8a/0xe0 [nvmet]
> ? __pfx_nvmet_ns_enable_store+0x10/0x10 [nvmet]
> configfs_write_iter+0x2ae/0x460
> vfs_write+0x540/0xd90
> ? __pfx_vfs_write+0x10/0x10
> ? __pfx___lock_acquire+0x10/0x10
> ? __handle_mm_fault+0x12c5/0x1870
> ? __fget_light+0x51/0x220
> ksys_write+0xf7/0x1d0
> ? __pfx_ksys_write+0x10/0x10
> ? syscall_enter_from_user_mode+0x22/0x90
> do_syscall_64+0x60/0xe0
> ? __pfx_lock_release+0x10/0x10
> ? count_memcg_events.constprop.0+0x4a/0x60
> ? handle_mm_fault+0x1b1/0x9d0
> ? exc_page_fault+0xc0/0x100
> ? rcu_is_watching+0x11/0xb0
> ? asm_exc_page_fault+0x22/0x30
> ? lockdep_hardirqs_on+0x7d/0x100
> entry_SYSCALL_64_after_hwframe+0x6e/0x76
> RIP: 0033:0x7f604525ac34
> Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa 80 3d 35 77 0d 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 c3 0f 1f 00 55 48 89 e5 48 83 ec 20 48 89
> RSP: 002b:00007ffec7fd6ce8 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f604525ac34
> RDX: 0000000000000002 RSI: 0000562b0cd805a0 RDI: 0000000000000001
> RBP: 00007ffec7fd6d10 R08: 0000000000001428 R09: 0000000100000000
> R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000002
> R13: 0000562b0cd805a0 R14: 00007f604532b5c0 R15: 00007f6045328f20
> </TASK>
Thanks a lot, I will fix this in my reservation patch set v5.
Best regards,
Guixin Liu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Re: [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument
2024-01-23 10:54 ` Shinichiro Kawasaki
@ 2024-01-31 14:47 ` Daniel Wagner
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Wagner @ 2024-01-31 14:47 UTC (permalink / raw)
To: Shinichiro Kawasaki
Cc: Guixin Liu, chaitanyak@nvidia.com, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org
On Tue, Jan 23, 2024 at 10:54:07AM +0000, Shinichiro Kawasaki wrote:
> On Jan 17, 2024 / 16:17, Guixin Liu wrote:
> > Add an optional argument --resv_enable to _nvmet_target_setup() and
> > propagate it to _create_nvmet_subsystem() and _create_nvmet_ns().
> >
> > One can call functions with --resv_enable to enable reservation
> > feature on a specific namespace.
> >
> > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>
> Thanks, looks good to me.
>
> Daniel, could you take a look in this patch? I think it is consistent with your
> work on _nvme_connect_subsys() and _nvmet_target_setup() in the past.
It teaches _create_nvmet_ns and _create_nvmet_subsystem to parse for
arguments as we have for the function you named. This makes these
function a bit more flexible to use. Looks good to me. Thanks!
Reviewed-by: Daniel Wagner <dwagner@suse.de>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-31 14:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-17 8:17 [PATCH V2 0/2] *** Test the NVMe reservation feature *** Guixin Liu
2024-01-17 8:17 ` [PATCH V2 1/2] nvme/{rc,002,016,017,030,031}: introduce --resv_enable argument Guixin Liu
2024-01-23 10:54 ` Shinichiro Kawasaki
2024-01-31 14:47 ` Daniel Wagner
2024-01-17 8:17 ` [PATCH V2 2/2] test/nvme/050: test the reservation feature Guixin Liu
2024-01-23 11:21 ` Shinichiro Kawasaki
2024-01-23 11:41 ` Guixin Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox