Skip to contentSkip to conversation

blktests: test ANA base support

PATCH Superseded Hannes Reinecke 5 participants 14 messages Jul 17, 2018 – Jul 26, 2018

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover blktests: test ANA base support

Hi all,

here's a small patchset for testing ANA base support. The test itself requires the ANA patchset from Christoph, plus the fixes I've sent to the mailing list earlier.

Hannes Reinecke (2):
blktests: enable ANA support
blktests: add test for ANA state transition

 tests/nvme/014     | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/014.out |  17 ++++++
 tests/nvme/rc      |  49 ++++++++++++++++-
 3 files changed, 221 insertions(+), 3 deletions(-)
 create mode 100755 tests/nvme/014
 create mode 100644 tests/nvme/014.out
Signature · 1 line
2.12.3
#1 lore raw

PATCH 1/2 blktests: enable ANA support

Update nvme functions to support ANA.

Signed-off-by Hannes Reinecke <hare at suse.com>
---
 tests/nvme/rc | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)
tests/nvme/rc +46 -3
index fb5dbdf..116661d 100644
@@ -27,7 +27,7 @@ group_device_requires() {
 	_test_dev_is_nvme
 }
 
-NVMET_CFS="/sys/kernel/config/nvmet/"
+NVMET_CFS="/sys/kernel/config/nvmet"
 
 _test_dev_is_nvme() {
 	if ! readlink -f "$TEST_DEV_SYSFS/device" | grep -q nvme; then
@@ -49,6 +49,7 @@ _create_nvmet_port() {
 
 	mkdir "${NVMET_CFS}/ports/${port}"
 	echo "${trtype}" > "${NVMET_CFS}/ports/${port}/addr_trtype"
+	echo "${port}" > "${NVMET_CFS}/ports/${port}/addr_traddr"
 
 	echo "${port}"
 }
@@ -58,6 +59,39 @@ _remove_nvmet_port() {
 	rmdir "${NVMET_CFS}/ports/${port}"
 }
 
+_create_nvmet_anagroup() {
+	local port="$1"
+	local port_cfs="${NVMET_CFS}/ports/${port}"
+	local grpid
+
+	for ((grpid = 1; ; grpid++)); do
+		if [[ ! -e "${port_cfs}/ana_groups/${grpid}" ]]; then
+			break
+		fi
+	done
+
+	mkdir "${port_cfs}/ana_groups/${grpid}"
+
+	echo "${grpid}"
+}
+
+_remove_nvmet_anagroup() {
+	local port="$1"
+	local grpid="$2"
+	local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+	rmdir "${ana_cfs}"
+}
+
+_set_nvmet_anagroup_state() {
+	local port="$1"
+	local grpid="$2"
+	local state="$3"
+	local ana_cfs="${NVMET_CFS}/ports/${port}/ana_groups/${grpid}"
+
+	echo "${state}" > "${ana_cfs}/ana_state"
+}
+
 _create_nvmet_ns() {
 	local nvmet_subsystem="$1"
 	local nsid="$2"
@@ -65,14 +99,22 @@ _create_nvmet_ns() {
 	local uuid="00000000-0000-0000-0000-000000000000"
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 	local ns_path="${subsys_path}/namespaces/${nsid}"
+	local ana_grpid
 
-	if [[ $# -eq 4 ]]; then
+	if [[ $# -ge 4 ]]; then
 		uuid="$4"
 	fi
 
+	if [[ $# -eq 5 ]]; then
+		ana_grpid="$5"
+	fi
+
 	mkdir "${ns_path}"
 	printf "%s" "${blkdev}" > "${ns_path}/device_path"
 	printf "%s" "${uuid}" > "${ns_path}/device_uuid"
+	if [ -n "${ana_grpid}" ] ; then
+		printf "%s" "${ana_grpid}" > "${ns_path}/ana_grpid"
+	fi
 	printf 1 > "${ns_path}/enable"
 }
 
@@ -80,11 +122,12 @@ _create_nvmet_subsystem() {
 	local nvmet_subsystem="$1"
 	local blkdev="$2"
 	local uuid=$3
+	local ana_grpid=$4
 	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 
 	mkdir -p "${cfs_path}"
 	echo 1 > "${cfs_path}/attr_allow_any_host"
-	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
+	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}" "${ana_grpid}"
 }
 
 _remove_nvmet_ns() {
Signature · 1 line
2.12.3
#2 lore raw

PATCH 2/2 blktests: add test for ANA state transition

In reply to Hannes Reinecke
Signed-off-by Hannes Reinecke <hare at suse.com>
---
 tests/nvme/014     | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/014.out |  17 ++++++
 2 files changed, 175 insertions(+)
 create mode 100755 tests/nvme/014
 create mode 100644 tests/nvme/014.out
tests/nvme/014 new file +158 -0
new file mode 100755
index 0000000..4b57229
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+	local port1="$1"
+	local port2="$2"
+	local mode="$3"
+
+	echo "ANA state ${mode}"
+
+	if [ "${mode}" = "change" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "change"
+		_set_nvmet_anagroup_state "${port1}" "2" "change"
+		_set_nvmet_anagroup_state "${port2}" "1" "change"
+		_set_nvmet_anagroup_state "${port2}" "2" "change"
+	elif [ "${mode}" = "failover" ] ; then
+		_set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port1}" "2" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+	else
+		_set_nvmet_anagroup_state "${port1}" "1" "optimized"
+		_set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+		_set_nvmet_anagroup_state "${port2}" "2" "optimized"
+	fi
+}
+
+_display_ana_state() {
+	local grpid state
+	for nvme in /sys/class/nvme/* ; do
+		for c in ${nvme}/nvme* ; do
+			if [ ! -d ${c} ] ; then
+				echo "${nvme##*/}: ANA disabled"
+				continue
+			fi
+			grpid="$(cat "${c}/ana_grpid")"
+			state="$(cat "${c}/ana_state")"
+			echo "${c##*/}: grpid ${grpid} state ${state}"
+		done
+	done
+}
+
+_switch_ana_states() {
+	local port1=$1
+	local port2=$2
+
+}
+
+requires() {
+	_have_program nvme && _have_module nvme-loop && _have_module loop && \
+		_have_configfs && _have_fio
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	modprobe nvmet
+	modprobe nvme-loop
+
+	local port1
+	port1="$(_create_nvmet_port "loop")"
+	ag1="$(_create_nvmet_anagroup "${port1}")"
+
+	local port2
+	port2="$(_create_nvmet_port "loop")"
+	ag2="$(_create_nvmet_anagroup "${port2}")"
+
+	truncate -s 1G "$TMPDIR/img1"
+
+	local loop_dev1
+	loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+	_create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
+		"91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+	truncate -s 1G "$TMPDIR/img2"
+
+	local loop_dev2
+	loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+	_create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
+		"9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+	_add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
+	_add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
+
+	switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+	nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
+	nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
+
+	_display_ana_state
+
+	_run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
+	trap "kill $!" EXIT
+
+	sleep 10
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state
+
+	sleep 6
+
+	switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+	# Insert a delay to allow the AEN to be processed
+	sleep 1
+
+	_display_ana_state
+
+	wait
+	trap - EXIT
+
+	nvme disconnect -n blktests-subsystem-1
+
+	_remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
+	_remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
+	_remove_nvmet_ns "blktests-subsystem-1" "2"
+	_remove_nvmet_subsystem "blktests-subsystem-1"
+	_remove_nvmet_anagroup "${port1}" "2"
+	_remove_nvmet_port "${port1}"
+	_remove_nvmet_anagroup "${port2}" "2"
+	_remove_nvmet_port "${port2}"
+	losetup -d "$loop_dev2"
+	rm "$TMPDIR/img2"
+	losetup -d "$loop_dev1"
+	rm "$TMPDIR/img1"
+
+	modprobe -r nvme-loop
+	modprobe -r nvmet
+	echo "Test complete"
+}
tests/nvme/014.out new file +17 -0
new file mode 100644
index 0000000..0a6ed35
@@ -0,0 +1,17 @@
+Running nvme/016
+nvme0c1n1: grpid 1 state optimized
+nvme0c1n2: grpid 2 state inaccessible
+nvme0c2n1: grpid 1 state inaccessible
+nvme0c2n2: grpid 2 state optimized
+ANA state transition
+nvme0c1n1: grpid 1 state change
+nvme0c1n2: grpid 2 state change
+nvme0c2n1: grpid 1 state change
+nvme0c2n2: grpid 2 state change
+ANA state failover
+nvme0c1n1: grpid 1 state inaccessible
+nvme0c1n2: grpid 2 state optimized
+nvme0c2n1: grpid 1 state optimized
+nvme0c2n2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
Signature · 1 line
2.12.3
#3 lore raw

Re: [PATCH 2/2] blktests: add test for ANA state transition

tests/nvme/014 line 74

Quoted 4 lines · Hannes Reinecke
tests/nvme/014 new file +4 -0
+requires() {
+	_have_program nvme && _have_module nvme-loop && _have_module loop && \
+		_have_configfs && _have_fio
+}

this needs '_have_module_param nvme-core multipath' as well.

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#4 lore raw

Re: [PATCH 2/2] blktests: add test for ANA state transition

In reply to Hannes Reinecke

From: [email protected] <[email protected]> on behalf of Hannes Reinecke <[email protected]>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 2/2] blktests: add test for ANA state transition
?

Signed-off-by Hannes Reinecke <hare at suse.com>

---
?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++ ?tests/nvme/014.out |? 17 ++++++
?2 files changed, 175 insertions(+)
?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

tests/nvme/014 new file +18 -0
new file mode 100755
index 0000000..4b57229
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.? If not, see <http://www.gnu.org/licenses/>.

Licenses - GNU Project - Free Software Foundation
www.gnu.org
Published software should be free software.To make it free software, you need to release it under a free software license. We normally use the GNU General Public License (GNU GPL), specifying version 3 or any later version, but occasionally we use other free software licenses.

+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support" +QUICK=1
+
+switch_nvmet_anagroup() {
+?????? local port1="$1"
+?????? local port2="$2"
+?????? local mode="$3"
+
+?????? echo "ANA state ${mode}"
+
+?????? if [ "${mode}" = "change" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "change" +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "change" +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "change" +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "change" +?????? elif [ "${mode}" = "failover" ] ; then +?????????????? _set_nvmet_anagroup_state "${port1}" "1" "inaccessible" +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "optimized" +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "optimized" +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "inaccessible" +?????? else
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "optimized" +?????????????? _set_nvmet_anagroup_state "${port1}" "2" "inaccessible" +?????????????? _set_nvmet_anagroup_state "${port2}" "1" "inaccessible" +?????????????? _set_nvmet_anagroup_state "${port2}" "2" "optimized" +?????? fi
+}
+
+_display_ana_state() {
+?????? local grpid state
[CK] Newliine here ?
+?????? for nvme in /sys/class/nvme/* ; do
+?????????????? for c in ${nvme}/nvme* ; do +?????????????????????? if [ ! -d ${c} ] ; then +?????????????????????????????? echo "${nvme##*/}: ANA disabled" +?????????????????????????????? continue +?????????????????????? fi
+?????????????????????? grpid="$(cat "${c}/ana_grpid")" +?????????????????????? state="$(cat "${c}/ana_state")" +?????????????????????? echo "${c##*/}: grpid ${grpid} state ${state}" +?????????????? done
+?????? done
+}

I think we need to move above functions to the ${BLKTESTS_HOME}/tests/nvme/rc. +
+_switch_ana_states() {
+?????? local port1=$1
+?????? local port2=$2
+
[CK] Please remove the extra line.
+}
[CK] I was not able to find a caller for above function, I'm I missing something ? +
+requires() {
+?????? _have_program nvme && _have_module nvme-loop && _have_module loop && \
+?????????????? _have_configfs && _have_fio
[CK] Missing nvmet module from the above list.
+}

Can we split following test function into small routines, it will be easier to review and maintain?

+
+test() {
+?????? echo "Running ${TEST_NAME}"
+
+?????? modprobe nvmet
+?????? modprobe nvme-loop
+
+?????? local port1
+?????? port1="$(_create_nvmet_port "loop")"
[CK] Can we initialize variables at the time of declaration or after declaration of all the variables ?
+?????? ag1="$(_create_nvmet_anagroup "${port1}")"
[CK] Not sure if we need ag1 variable.

+
+?????? local port2
[CK] Can we plese declare all the variable at the top please see tests/nvme/006-013 to maintain uniform style ?
+?????? port2="$(_create_nvmet_port "loop")"
+?????? ag2="$(_create_nvmet_anagroup "${port2}")"
+
+?????? truncate -s 1G "$TMPDIR/img1"
+
+?????? local loop_dev1
+?????? loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+?????? _create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
+?????????????? "91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+?????? truncate -s 1G "$TMPDIR/img2"
+
[CK] Do we really need img2 ? is it possible to use the img1 as we are not doing data verification ? +?????? local loop_dev2
+?????? loop_dev2="$(losetup -f --show "$TMPDIR/img2")"
+
+?????? _create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
+?????????????? "9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+?????? _add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
+?????? _add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+?????? nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
+?????? nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
+
+?????? _display_ana_state
+
+?????? _run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
[CK] Hardcoded value for ns, on my machine I have PCIe ns with "/dev/nvme0n1",
can we please make sure this is the right block device for the namespace that we just configured ?
+?????? trap "kill $!" EXIT
+
+?????? sleep 10
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? sleep 6
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? wait
+?????? trap - EXIT
+
+?????? nvme disconnect -n blktests-subsystem-1
+
+?????? _remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
+?????? _remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
+?????? _remove_nvmet_ns "blktests-subsystem-1" "2"
+?????? _remove_nvmet_subsystem "blktests-subsystem-1"
+?????? _remove_nvmet_anagroup "${port1}" "2"
+?????? _remove_nvmet_port "${port1}"
+?????? _remove_nvmet_anagroup "${port2}" "2"
[CK] Plase make the groupid local varible instead of hard coding "2".
+?????? _remove_nvmet_port "${port2}"
+?????? losetup -d "$loop_dev2"
+?????? rm "$TMPDIR/img2"
+?????? losetup -d "$loop_dev1"
+?????? rm "$TMPDIR/img1"
+
+?????? modprobe -r nvme-loop
+?????? modprobe -r nvmet
+?????? echo "Test complete"
+}

tests/nvme/014.out new file +17 -0
new file mode 100644
index 0000000..0a6ed35
@@ -0,0 +1,17 @@
+Running nvme/016
+nvme0c1n1: grpid 1 state optimized
+nvme0c1n2: grpid 2 state inaccessible
+nvme0c2n1: grpid 1 state inaccessible
+nvme0c2n2: grpid 2 state optimized
+ANA state transition
+nvme0c1n1: grpid 1 state change
+nvme0c1n2: grpid 2 state change
+nvme0c2n1: grpid 1 state change
+nvme0c2n2: grpid 2 state change
+ANA state failover
+nvme0c1n1: grpid 1 state inaccessible
+nvme0c1n2: grpid 2 state optimized
+nvme0c2n1: grpid 1 state optimized
+nvme0c2n2: grpid 2 state inaccessible
+NQN:blktests-subsystem-1 disconnected 2 controller(s)
+Test complete
Signature · 1 line
2.12.3
#5 lore raw

Re: [PATCH 2/2] blktests: add test for ANA state transition

Quoted 41 lines · Chaitanya Kulkarni

From: linux-block-owner at vger.kernel.org <linux-block-owner at vger.kernel.org> on behalf of Hannes Reinecke <hare at suse.de>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke; Hannes Reinecke
Subject: [PATCH 2/2] blktests: add test for ANA state transition

Signed-off-by Hannes Reinecke <hare at suse.com>
---
 ?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++

?tests/nvme/014.out |? 17 ++++++
?2 files changed, 175 insertions(+)
?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

tests/nvme/014 new file +18 -0
new file mode 100755
index 0000000..4b57229
@@ -0,0 +1,158 @@
+#!/bin/bash
+#
+# Regression test for ANA base support
+#
+# Copyright (C) 2018 Hannes Reinecke
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.? See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.? If not, see <http://www.gnu.org/licenses/>.
Quoted 6 lines

Licenses - GNU Project - Free Software Foundation
www.gnu.org
Published software should be free software.To make it free software, you need to release it under a free software license. We normally use the GNU General Public License (GNU GPL), specifying version 3 or any later version, but occasionally we use other free software licenses.

I don't mind, I just copied it over from testcase 10...

Quoted 33 lines
tests/nvme/014 new file +33 -0
+
+. tests/nvme/rc
+
+DESCRIPTION="test ANA optimized/transitioning/inaccessible support"
+QUICK=1
+
+switch_nvmet_anagroup() {
+?????? local port1="$1"
+?????? local port2="$2"
+?????? local mode="$3"
+
+?????? echo "ANA state ${mode}"
+
+?????? if [ "${mode}" = "change" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "change"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "change"
+?????? elif [ "${mode}" = "failover" ] ; then
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "inaccessible"
+?????? else
+?????????????? _set_nvmet_anagroup_state "${port1}" "1" "optimized"
+?????????????? _set_nvmet_anagroup_state "${port1}" "2" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "1" "inaccessible"
+?????????????? _set_nvmet_anagroup_state "${port2}" "2" "optimized"
+?????? fi
+}
+
+_display_ana_state() {
+?????? local grpid state
Quoted 1 line

[CK] Newliine here ?

Okay

Quoted 13 lines

+?????? for nvme in /sys/class/nvme/* ; do
+?????????????? for c in ${nvme}/nvme* ; do +?????????????????????? if [ ! -d ${c} ] ; then +?????????????????????????????? echo "${nvme##*/}: ANA disabled" +?????????????????????????????? continue +?????????????????????? fi
+?????????????????????? grpid="$(cat "${c}/ana_grpid")" +?????????????????????? state="$(cat "${c}/ana_state")" +?????????????????????? echo "${c##*/}: grpid ${grpid} state ${state}" +?????????????? done
+?????? done
+}

Quoted 1 line

I think we need to move above functions to the ${BLKTESTS_HOME}/tests/nvme/rc.

Okay, can do.

Quoted 9 lines
tests/nvme/014 new file +5 -0
+
+_switch_ana_states() {
+?????? local port1=$1
+?????? local port2=$2
+

[CK] Please remove the extra line.
+}
[CK] I was not able to find a caller for above function, I'm I missing something ? +

Yeah, that's an older function which got left over during refactoring.

Quoted 18 lines

+requires() {
+?????? _have_program nvme && _have_module nvme-loop && _have_module loop && \
+?????????????? _have_configfs && _have_fio
[CK] Missing nvmet module from the above list.
+}

Can we split following test function into small routines, it will be easier to review and maintain?

tests/nvme/014 new file +9 -0
+
+test() {
+?????? echo "Running ${TEST_NAME}"
+
+?????? modprobe nvmet
+?????? modprobe nvme-loop
+
+?????? local port1
+?????? port1="$(_create_nvmet_port "loop")"
Quoted 2 lines

[CK] Can we initialize variables at the time of declaration or after declaration of all the variables ?

Sure. But we should do it consistently; the older tests also do not follow these rules...

Quoted 3 lines

+?????? ag1="$(_create_nvmet_anagroup "${port1}")" [CK] Not sure if we need ag1 variable.

Yeah, it's more for symmetry than anything else.

Quoted 4 lines

+
+?????? local port2
[CK] Can we plese declare all the variable at the top please see tests/nvme/006-013 to maintain uniform style ?

Sure. But see above; not all tests follow the same scheme.

Quoted 13 lines

+?????? port2="$(_create_nvmet_port "loop")" +?????? ag2="$(_create_nvmet_anagroup "${port2}")"

tests/nvme/014 new file +11 -0
+
+?????? truncate -s 1G "$TMPDIR/img1"
+
+?????? local loop_dev1
+?????? loop_dev1="$(losetup -f --show "$TMPDIR/img1")"
+
+?????? _create_nvmet_subsystem "blktests-subsystem-1" "${loop_dev1}" \
+?????????????? "91fdba0d-f87b-4c25-b80f-db7be1418b9e" "1"
+
+?????? truncate -s 1G "$TMPDIR/img2"
+
Quoted 1 line

[CK] Do we really need img2 ? is it possible to use the img1 as we are not doing data verification ?

Yes. There's an issue with the current implementation which fails to signal a namespace state change for the _second_ namepsace.

(That's the one patch I've sent to the current ANA implementation from hch)

Quoted 17 lines

+?????? local loop_dev2
+?????? loop_dev2="$(losetup -f --show "$TMPDIR/img2")"

tests/nvme/014 new file +15 -0
+
+?????? _create_nvmet_ns "blktests-subsystem-1" "2" "${loop_dev2}" \
+?????????????? "9aed0138-bfd9-46f5-92ac-24c70377fd49" "2"
+
+?????? _add_nvmet_subsys_to_port "${port1}" "blktests-subsystem-1"
+?????? _add_nvmet_subsys_to_port "${port2}" "blktests-subsystem-1"
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" failback
+
+?????? nvme connect -t loop -a "${port1}" -n blktests-subsystem-1
+?????? nvme connect -t loop -a "${port2}" -n blktests-subsystem-1
+
+?????? _display_ana_state
+
+?????? _run_fio_rand_io --size=256m --filename="/dev/nvme0n1" &
Quoted 3 lines

[CK] Hardcoded value for ns, on my machine I have PCIe ns with "/dev/nvme0n1", can we please make sure this is the right block device for the namespace that we just configured ?

Yeah, I know. Will see what I can do.

Quoted 32 lines

+?????? trap "kill $!" EXIT

tests/nvme/014 new file +31 -0
+
+?????? sleep 10
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "change"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? sleep 6
+
+?????? switch_nvmet_anagroup "${port1}" "${port2}" "failover"
+
+?????? # Insert a delay to allow the AEN to be processed
+?????? sleep 1
+
+?????? _display_ana_state
+
+?????? wait
+?????? trap - EXIT
+
+?????? nvme disconnect -n blktests-subsystem-1
+
+?????? _remove_nvmet_subsystem_from_port "${port1}" "blktests-subsystem-1"
+?????? _remove_nvmet_subsystem_from_port "${port2}" "blktests-subsystem-1"
+?????? _remove_nvmet_ns "blktests-subsystem-1" "2"
+?????? _remove_nvmet_subsystem "blktests-subsystem-1"
+?????? _remove_nvmet_anagroup "${port1}" "2"
+?????? _remove_nvmet_port "${port1}"
+?????? _remove_nvmet_anagroup "${port2}" "2"
Quoted 1 line

[CK] Plase make the groupid local varible instead of hard coding "2".

Hmm. Not easy. But I'll have a look.

Thanks for the review.

Cheers,

Hannes

#6 lore raw

cover blktests: test ANA base support

In reply to Hannes Reinecke

Hi Hannes,

Thanks for the test, this is really helpful, please see my inline comments to individual patches.

Also, the test number should be I guess 18, 14-17 testcases are still in the queue.

Omar, can you please let us know when can you merge the remaining testcases for NVMeOF from the first round that I've posted? so we can set the right number for this.

From: [email protected] <[email protected]> on behalf of Hannes Reinecke <[email protected]>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke
Subject: [PATCH 0/2] blktests: test ANA base support
?

Hi all,

here's a small patchset for testing ANA base support. The test itself requires the ANA patchset from Christoph, plus the fixes I've sent to the mailing list earlier.

Hannes Reinecke (2):
? blktests: enable ANA support
? blktests: add test for ANA state transition

?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
?tests/nvme/014.out |? 17 ++++++
?tests/nvme/rc????? |? 49 ++++++++++++++++-

?3 files changed, 221 insertions(+), 3 deletions(-) ?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

Signature · 1 line
2.12.3
#7 lore raw

cover blktests: test ANA base support

Quoted 5 lines · Chaitanya Kulkarni

Hi Hannes,

Thanks for the test, this is really helpful, please see my inline comments to individual patches.

Also, the test number should be I guess 18, 14-17 testcases are still in the queue.

Yeah, sorry about that, I'll take another look.

Quoted 2 lines

Omar, can you please let us know when can you merge the remaining testcases for NVMeOF from the first round that I've posted? so we can set the right number for this.

No need to worry about the numbering, I can trivially renumber it here.

Quoted 29 lines

From: linux-block-owner at vger.kernel.org <linux-block-owner at vger.kernel.org> on behalf of Hannes Reinecke <hare at suse.de>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke
Subject: [PATCH 0/2] blktests: test ANA base support
?

Hi all,

here's a small patchset for testing ANA base support. The test itself requires the ANA patchset from Christoph, plus the fixes I've sent to the mailing list earlier.

Hannes Reinecke (2):
? blktests: enable ANA support
? blktests: add test for ANA state transition

?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
?tests/nvme/014.out |? 17 ++++++
?tests/nvme/rc????? |? 49 ++++++++++++++++-

?3 files changed, 221 insertions(+), 3 deletions(-) ?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

Signature · 1 line
2.12.3
#8 lore raw

cover blktests: test ANA base support

Thanks, Omar.

From: Omar Sandoval <[email protected]>
Sent: Wednesday, July 25, 2018 12:17 PM
To: Chaitanya Kulkarni
Cc: Hannes Reinecke; Omar Sandoval; Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org
Subject: Re: [PATCH 0/2] blktests: test ANA base support
?

Quoted 5 lines · Chaitanya Kulkarni

Hi Hannes,

Thanks for the test, this is really helpful, please see my inline comments to individual patches.

Also, the test number should be I guess 18, 14-17 testcases are still in the queue.

Yeah, sorry about that, I'll take another look.

Quoted 2 lines

Omar, can you please let us know when can you merge the remaining testcases for NVMeOF from the first round that I've posted? so we can set the right number for this.

No need to worry about the numbering, I can trivially renumber it here.

Quoted 29 lines

From: linux-block-owner at vger.kernel.org <linux-block-owner at vger.kernel.org> on behalf of Hannes Reinecke <hare at suse.de>
Sent: Tuesday, July 17, 2018 6:31 AM
To: Omar Sandoval
Cc: Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org; Hannes Reinecke
Subject: [PATCH 0/2] blktests: test ANA base support
?
?
Hi all,

here's a small patchset for testing ANA base support. The test itself requires the ANA patchset from Christoph, plus the fixes I've sent to the mailing list earlier.

Hannes Reinecke (2):
? blktests: enable ANA support
? blktests: add test for ANA state transition

?tests/nvme/014???? | 158 +++++++++++++++++++++++++++++++++++++++++++++++++++++
?tests/nvme/014.out |? 17 ++++++
?tests/nvme/rc????? |? 49 ++++++++++++++++-

?3 files changed, 221 insertions(+), 3 deletions(-) ?create mode 100755 tests/nvme/014
?create mode 100644 tests/nvme/014.out

Signature · 3 lines
2.12.3

???? 
#9 lore raw

cover blktests: test ANA base support

Quoted 2 lines · Chaitanya Kulkarni

Thanks, Omar.

Tests nvme/014 and nvme/015 had a pretty bad typo that I didn't notice last time:

dd=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k

That should be

dd if=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k status=none

When I fix that (and change the nvme flush call as mentioned before), I sometimes get a hung task:

[  273.844440] run blktests nvme/015 at 2018-07-25 13:44:11
[  273.861950] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[  273.875014] nvmet: creating controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:c5e36fdf-8e4d-4c27-be56-da373db583b2.
[  273.877457] nvme nvme1: creating 4 I/O queues.
[  273.879141] nvme nvme1: new ctrl: "blktests-subsystem-1"
[  276.247708] nvme nvme1: using deprecated NVME_IOCTL_IO_CMD ioctl on the char device!
[  276.262835] nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1"
[  289.755361] nvmet: ctrl 1 keep-alive timer (15 seconds) expired!
[  289.760579] nvmet: ctrl 1 fatal error occurred!
[  491.095890] INFO: task kworker/u8:0:7 blocked for more than 120 seconds.
[  491.104407]       Not tainted 4.18.0-rc6 #18
[  491.108330] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  491.116521] kworker/u8:0    D    0     7      2 0x80000000
[  491.121754] Workqueue: nvme-delete-wq nvme_delete_ctrl_work [nvme_core]
[  491.129604] Call Trace:
[  491.131611]  ? __schedule+0x2a1/0x890
[  491.135112]  ? _raw_spin_unlock_irqrestore+0x20/0x40
[  491.140542]  schedule+0x32/0x90
[  491.142030]  blk_mq_freeze_queue_wait+0x41/0xa0
[  491.144186]  ? wait_woken+0x80/0x80
[  491.145726]  blk_cleanup_queue+0x75/0x160
[  491.150235]  nvme_ns_remove+0xf9/0x130 [nvme_core]
[  491.151910]  nvme_remove_namespaces+0x86/0xc0 [nvme_core]
[  491.153127]  nvme_delete_ctrl_work+0x4b/0x80 [nvme_core]
[  491.154727]  process_one_work+0x18c/0x360
[  491.155428]  worker_thread+0x1c6/0x380
[  491.156160]  ? process_one_work+0x360/0x360
[  491.157493]  kthread+0x112/0x130
[  491.159119]  ? kthread_flush_work_fn+0x10/0x10
[  491.160008]  ret_from_fork+0x35/0x40
[  491.160729] INFO: task nvme:1139 blocked for more than 120 seconds.
[  491.162416]       Not tainted 4.18.0-rc6 #18
[  491.164348] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  491.166012] nvme            D    0  1139   1072 0x00000000
[  491.167946] Call Trace:
[  491.168459]  ? __schedule+0x2a1/0x890
[  491.169312]  schedule+0x32/0x90
[  491.170180]  schedule_timeout+0x311/0x4a0
[  491.171921]  ? kernfs_fop_release+0xa0/0xa0
[  491.172884]  wait_for_common+0x1a0/0x1d0
[  491.173813]  ? wake_up_q+0x70/0x70
[  491.174410]  flush_work+0x10e/0x1c0
[  491.174991]  ? flush_workqueue_prep_pwqs+0x130/0x130
[  491.176113]  nvme_delete_ctrl_sync+0x41/0x50 [nvme_core]
[  491.177969]  nvme_sysfs_delete+0x28/0x30 [nvme_core]
[  491.178632]  kernfs_fop_write+0x116/0x190
[  491.179254]  __vfs_write+0x36/0x190
[  491.179812]  vfs_write+0xa9/0x190
[  491.180344]  ksys_write+0x4f/0xb0
[  491.181056]  do_syscall_64+0x5b/0x170
[  491.181583]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  491.182311] RIP: 0033:0x7fc04176b9d4
[  491.182863] Code: Bad RIP value.
[  491.183650] RSP: 002b:00007ffc33bd15a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[  491.184622] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007fc04176b9d4
[  491.185606] RDX: 0000000000000001 RSI: 000055884bd0810a RDI: 0000000000000004
[  491.186719] RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000f
[  491.187684] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[  491.189045] R13: 00007ffc33bd2c20 R14: 00007ffc33bd15f0 R15: 0000000000000010

I pushed what I have to https://github.com/osandov/blktests/tree/nvmeof,
can you please take a look?

#10 lore raw

cover blktests: test ANA base support

Thanks for the report and correction. I was able to run the test and reproduce the problem, I also confirm that it is not that consistent.

As I suspected the problem is in the nvme disconnect in the following call chain:-

nvme_sysfs_delete
?nvme_delete_ctrl_sync
? ?nvme_delete_ctrl_work
? ? nvme_remove_namespaces
? ? ?nvme_ns_remove
? ? ? nvme_ns_remove
? ? ? ?? blk_cleanup_queue
? ? ? ? blk_freeze_queue
? ? ? ? ?? blk_freeze_queue_start
? ? ? ? ? wait_event(q->mq_freeze_wq, percpu_ref_is_zero(&q->q_usage_counter));?<----- Stuck here.
? ? ? ??
blk_cleanup_queue()?that's where it is blocking and then target ctrl is timing out on keep alive.

It will take some time for me to debug and find a fix let's merge this test once we fix this issue.

How should we go about other tests ? should we merge them and keep this one out in the nvmeof branch? Regards,
Chaitanya

From: Omar Sandoval <[email protected]>
Sent: Wednesday, July 25, 2018 2:00 PM
To: Chaitanya Kulkarni
Cc: Hannes Reinecke; Omar Sandoval; Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org
Subject: Re: [PATCH 0/2] blktests: test ANA base support
?

Quoted 2 lines · Chaitanya Kulkarni

Thanks, Omar.

Tests nvme/014 and nvme/015 had a pretty bad typo that I didn't notice last time:

dd=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k

That should be

dd if=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k status=none

When I fix that (and change the nvme flush call as mentioned before), I sometimes get a hung task:

[? 273.844440] run blktests nvme/015 at 2018-07-25 13:44:11
[? 273.861950] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[? 273.875014] nvmet: creating controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:c5e36fdf-8e4d-4c27-be56-da373db583b2. [? 273.877457] nvme nvme1: creating 4 I/O queues.
[? 273.879141] nvme nvme1: new ctrl: "blktests-subsystem-1"
[? 276.247708] nvme nvme1: using deprecated NVME_IOCTL_IO_CMD ioctl on the char device!
[? 276.262835] nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1"
[? 289.755361] nvmet: ctrl 1 keep-alive timer (15 seconds) expired!
[? 289.760579] nvmet: ctrl 1 fatal error occurred!
[? 491.095890] INFO: task kworker/u8:0:7 blocked for more than 120 seconds.
[? 491.104407]?????? Not tainted 4.18.0-rc6 #18
[? 491.108330] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[? 491.116521] kworker/u8:0??? D??? 0???? 7????? 2 0x80000000
[? 491.121754] Workqueue: nvme-delete-wq nvme_delete_ctrl_work [nvme_core]
[? 491.129604] Call Trace:
[? 491.131611]? ? __schedule+0x2a1/0x890
[? 491.135112]? ? _raw_spin_unlock_irqrestore+0x20/0x40
[? 491.140542]? schedule+0x32/0x90
[? 491.142030]? blk_mq_freeze_queue_wait+0x41/0xa0
[? 491.144186]? ? wait_woken+0x80/0x80
[? 491.145726]? blk_cleanup_queue+0x75/0x160
[? 491.150235]? nvme_ns_remove+0xf9/0x130 [nvme_core]
[? 491.151910]? nvme_remove_namespaces+0x86/0xc0 [nvme_core]
[? 491.153127]? nvme_delete_ctrl_work+0x4b/0x80 [nvme_core]
[? 491.154727]? process_one_work+0x18c/0x360
[? 491.155428]? worker_thread+0x1c6/0x380
[? 491.156160]? ? process_one_work+0x360/0x360
[? 491.157493]? kthread+0x112/0x130
[? 491.159119]? ? kthread_flush_work_fn+0x10/0x10
[? 491.160008]? ret_from_fork+0x35/0x40
[? 491.160729] INFO: task nvme:1139 blocked for more than 120 seconds.
[? 491.162416]?????? Not tainted 4.18.0-rc6 #18
[? 491.164348] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[? 491.166012] nvme??????????? D??? 0? 1139?? 1072 0x00000000
[? 491.167946] Call Trace:
[? 491.168459]? ? __schedule+0x2a1/0x890
[? 491.169312]? schedule+0x32/0x90
[? 491.170180]? schedule_timeout+0x311/0x4a0
[? 491.171921]? ? kernfs_fop_release+0xa0/0xa0
[? 491.172884]? wait_for_common+0x1a0/0x1d0
[? 491.173813]? ? wake_up_q+0x70/0x70
[? 491.174410]? flush_work+0x10e/0x1c0
[? 491.174991]? ? flush_workqueue_prep_pwqs+0x130/0x130
[? 491.176113]? nvme_delete_ctrl_sync+0x41/0x50 [nvme_core]
[? 491.177969]? nvme_sysfs_delete+0x28/0x30 [nvme_core]
[? 491.178632]? kernfs_fop_write+0x116/0x190
[? 491.179254]? __vfs_write+0x36/0x190
[? 491.179812]? vfs_write+0xa9/0x190
[? 491.180344]? ksys_write+0x4f/0xb0
[? 491.181056]? do_syscall_64+0x5b/0x170
[? 491.181583]? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[? 491.182311] RIP: 0033:0x7fc04176b9d4
[? 491.182863] Code: Bad RIP value.
[? 491.183650] RSP: 002b:00007ffc33bd15a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[? 491.184622] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007fc04176b9d4
[? 491.185606] RDX: 0000000000000001 RSI: 000055884bd0810a RDI: 0000000000000004
[? 491.186719] RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000f
[? 491.187684] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[? 491.189045] R13: 00007ffc33bd2c20 R14: 00007ffc33bd15f0 R15: 0000000000000010

I pushed what I have to https://github.com/osandov/blktests/tree/nvmeof,
can you please take a look?

#11 lore raw

cover blktests: test ANA base support

make sure this fix has been picked up in your kernel:
http://git.infradead.org/nvme.git/commit/6cdefc6e2ad52170f89a8d0e8b1a1339f91834dc

deletes were broken otherwise and have the exact trace below.

-- james

Quoted 116 lines · Chaitanya Kulkarni

Thanks for the report and correction. I was able to run the test and reproduce the problem, I also confirm that it is not that consistent.

As I suspected the problem is in the nvme disconnect in the following call chain:-

nvme_sysfs_delete
?nvme_delete_ctrl_sync
? ?nvme_delete_ctrl_work
? ? nvme_remove_namespaces
? ? ?nvme_ns_remove
? ? ? nvme_ns_remove
? ? ? ?? blk_cleanup_queue
? ? ? ? blk_freeze_queue
? ? ? ? ?? blk_freeze_queue_start
? ? ? ? ? wait_event(q->mq_freeze_wq, percpu_ref_is_zero(&q->q_usage_counter));?<----- Stuck here.

blk_cleanup_queue()?that's where it is blocking and then target ctrl is timing out on keep alive.

It will take some time for me to debug and find a fix let's merge this test once we fix this issue.

How should we go about other tests ? should we merge them and keep this one out in the nvmeof branch? Regards,
Chaitanya

From: Omar Sandoval <osandov at osandov.com>
Sent: Wednesday, July 25, 2018 2:00 PM
To: Chaitanya Kulkarni
Cc: Hannes Reinecke; Omar Sandoval; Christoph Hellwig; Sagi Grimberg; Keith Busch; linux-nvme at lists.infradead.org; linux-block at vger.kernel.org
Subject: Re: [PATCH 0/2] blktests: test ANA base support

Quoted 1 line · Chaitanya Kulkarni

Thanks, Omar.

Tests nvme/014 and nvme/015 had a pretty bad typo that I didn't notice last time:

dd=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k

That should be

dd if=/dev/urandom of="/dev/${nvmedev}n1" count=128000 bs=4k status=none

When I fix that (and change the nvme flush call as mentioned before), I sometimes get a hung task:

[? 273.844440] run blktests nvme/015 at 2018-07-25 13:44:11
[? 273.861950] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[? 273.875014] nvmet: creating controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:c5e36fdf-8e4d-4c27-be56-da373db583b2. [? 273.877457] nvme nvme1: creating 4 I/O queues.
[? 273.879141] nvme nvme1: new ctrl: "blktests-subsystem-1"
[? 276.247708] nvme nvme1: using deprecated NVME_IOCTL_IO_CMD ioctl on the char device!
[? 276.262835] nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1"
[? 289.755361] nvmet: ctrl 1 keep-alive timer (15 seconds) expired!
[? 289.760579] nvmet: ctrl 1 fatal error occurred!
[? 491.095890] INFO: task kworker/u8:0:7 blocked for more than 120 seconds.
[? 491.104407]?????? Not tainted 4.18.0-rc6 #18
[? 491.108330] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[? 491.116521] kworker/u8:0??? D??? 0???? 7????? 2 0x80000000
[? 491.121754] Workqueue: nvme-delete-wq nvme_delete_ctrl_work [nvme_core]
[? 491.129604] Call Trace:
[? 491.131611]? ? __schedule+0x2a1/0x890
[? 491.135112]? ? _raw_spin_unlock_irqrestore+0x20/0x40
[? 491.140542]? schedule+0x32/0x90
[? 491.142030]? blk_mq_freeze_queue_wait+0x41/0xa0
[? 491.144186]? ? wait_woken+0x80/0x80
[? 491.145726]? blk_cleanup_queue+0x75/0x160
[? 491.150235]? nvme_ns_remove+0xf9/0x130 [nvme_core]
[? 491.151910]? nvme_remove_namespaces+0x86/0xc0 [nvme_core]
[? 491.153127]? nvme_delete_ctrl_work+0x4b/0x80 [nvme_core]
[? 491.154727]? process_one_work+0x18c/0x360
[? 491.155428]? worker_thread+0x1c6/0x380
[? 491.156160]? ? process_one_work+0x360/0x360
[? 491.157493]? kthread+0x112/0x130
[? 491.159119]? ? kthread_flush_work_fn+0x10/0x10
[? 491.160008]? ret_from_fork+0x35/0x40
[? 491.160729] INFO: task nvme:1139 blocked for more than 120 seconds.
[? 491.162416]?????? Not tainted 4.18.0-rc6 #18
[? 491.164348] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[? 491.166012] nvme??????????? D??? 0? 1139?? 1072 0x00000000
[? 491.167946] Call Trace:
[? 491.168459]? ? __schedule+0x2a1/0x890
[? 491.169312]? schedule+0x32/0x90
[? 491.170180]? schedule_timeout+0x311/0x4a0
[? 491.171921]? ? kernfs_fop_release+0xa0/0xa0
[? 491.172884]? wait_for_common+0x1a0/0x1d0
[? 491.173813]? ? wake_up_q+0x70/0x70
[? 491.174410]? flush_work+0x10e/0x1c0
[? 491.174991]? ? flush_workqueue_prep_pwqs+0x130/0x130
[? 491.176113]? nvme_delete_ctrl_sync+0x41/0x50 [nvme_core]
[? 491.177969]? nvme_sysfs_delete+0x28/0x30 [nvme_core]
[? 491.178632]? kernfs_fop_write+0x116/0x190
[? 491.179254]? __vfs_write+0x36/0x190
[? 491.179812]? vfs_write+0xa9/0x190
[? 491.180344]? ksys_write+0x4f/0xb0
[? 491.181056]? do_syscall_64+0x5b/0x170
[? 491.181583]? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[? 491.182311] RIP: 0033:0x7fc04176b9d4
[? 491.182863] Code: Bad RIP value.
[? 491.183650] RSP: 002b:00007ffc33bd15a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[? 491.184622] RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007fc04176b9d4
[? 491.185606] RDX: 0000000000000001 RSI: 000055884bd0810a RDI: 0000000000000004
[? 491.186719] RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000f
[? 491.187684] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
[? 491.189045] R13: 00007ffc33bd2c20 R14: 00007ffc33bd15f0 R15: 0000000000000010

I pushed what I have to https://github.com/osandov/blktests/tree/nvmeof,
can you please take a look?

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

#13 lore raw
Quoted 10 lines · Omar Sandoval
Quoted 6 lines · James Smart

make sure this fix has been picked up in your kernel:
http://git.infradead.org/nvme.git/commit/6cdefc6e2ad52170f89a8d0e8b1a1339f91834dc

deletes were broken otherwise and have the exact trace below.

-- james

Thanks, James, that indeed fixes the problem. I'll go ahead and merge these tests since there's a fix in flight.

By "these tests", I mean Chaitanya's tests, Hannes still has some comments to resolve :)