* [PATCH blktests] test/nvme/050: test the reservation feature @ 2024-01-14 9:26 Guixin Liu 2024-01-15 5:15 ` Shinichiro Kawasaki 0 siblings, 1 reply; 3+ messages in thread From: Guixin Liu @ 2024-01-14 9:26 UTC (permalink / raw) To: shinichiro.kawasaki; +Cc: hch, sagi, kch, chaitanyak, linux-nvme, linux-block Test the reservation feature, includes register, acquire, release and report. Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> --- tests/nvme/050 | 67 ++++++++++++++++++++++++++ tests/nvme/050.out | 114 +++++++++++++++++++++++++++++++++++++++++++++ tests/nvme/rc | 3 ++ 3 files changed, 184 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..a499f66 --- /dev/null +++ b/tests/nvme/050 @@ -0,0 +1,67 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2024 Guixin Liu +# Copyright (C) 2024 Alibaba Group. +# +# Test the reservation +# +. tests/nvme/rc + +DESCRIPTION="test the reservation" +QUICK=1 + +requires() { + _nvme_requires +} + +test() { + echo "Running ${TEST_NAME}" + + _setup_nvmet + + local nvmedev + + _nvmet_target_setup --blkdev file + + _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" + + nvmedev=$(_find_nvme_dev "${def_subsysnqn}") + + echo "Register" + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Replace" + nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Unregister" + nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Acquire" + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Preempt" + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Release" + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + + echo "Clear" + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1 + + _nvme_disconnect_subsys "${def_subsysnqn}" + + _nvmet_target_cleanup + + echo "Test complete" +} diff --git a/tests/nvme/050.out b/tests/nvme/050.out new file mode 100644 index 0000000..3be417d --- /dev/null +++ b/tests/nvme/050.out @@ -0,0 +1,114 @@ +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 + hostid : f1fb429f7f4856b0b351e6b8de349 + +Replace +NVME Reservation success + +NVME Reservation status: + +gen : 2 +rtype : 0 +regctl : 1 +ptpls : 0 +regctlext[0] : + cntlid : 1 + rcsts : 0 + rkey : 5 + hostid : f1fb429f7f4856b0b351e6b8de349 + +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 + hostid : f1fb429f7f4856b0b351e6b8de349 + +Preempt +NVME Reservation Acquire success + +NVME Reservation status: + +gen : 5 +rtype : 2 +regctl : 1 +ptpls : 0 +regctlext[0] : + cntlid : 1 + rcsts : 1 + rkey : 4 + hostid : f1fb429f7f4856b0b351e6b8de349 + +Release +NVME Reservation Release success + +NVME Reservation status: + +gen : 5 +rtype : 0 +regctl : 1 +ptpls : 0 +regctlext[0] : + cntlid : 1 + rcsts : 0 + rkey : 4 + hostid : f1fb429f7f4856b0b351e6b8de349 + +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 + hostid : f1fb429f7f4856b0b351e6b8de349 + +NVME Reservation Release success +disconnected 1 controller(s) +Test complete diff --git a/tests/nvme/rc b/tests/nvme/rc index b0ef1ee..8de59e2 100644 --- a/tests/nvme/rc +++ b/tests/nvme/rc @@ -670,6 +670,9 @@ _create_nvmet_ns() { mkdir "${ns_path}" printf "%s" "${blkdev}" > "${ns_path}/device_path" printf "%s" "${uuid}" > "${ns_path}/device_uuid" + if [[ -f "${ns_path}/resv_enable" ]]; then + printf 1 > "${ns_path}/resv_enable" + fi printf 1 > "${ns_path}/enable" } -- 2.30.1 (Apple Git-130) ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH blktests] test/nvme/050: test the reservation feature 2024-01-14 9:26 [PATCH blktests] test/nvme/050: test the reservation feature Guixin Liu @ 2024-01-15 5:15 ` Shinichiro Kawasaki 2024-01-16 1:57 ` Guixin Liu 0 siblings, 1 reply; 3+ messages in thread From: Shinichiro Kawasaki @ 2024-01-15 5:15 UTC (permalink / raw) To: Guixin Liu Cc: hch@lst.de, sagi@grimberg.me, kch@nvidia.com, chaitanyak@nvidia.com, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org Hello Guixin, thanks for the patch. Good to have new tests for new features :) I think this test depends on the kernel side patches you posted [1]. So the test case should be skipped when the kernel does not have the patches. I suggest to check it using "resv_enable" configfs file. Please refer to nvme/048 which does similar check and sets SKIP_REASONS. [1] https://lore.kernel.org/linux-nvme/20240114092314.63694-1-kanie@linux.alibaba.com/ When I ran the test case on my system using the kernel v6.7 + the [1] patches, I observed the kernel BUG below. It looks kmalloc() in nvmet_execute_pr_report() needs care. [ 262.402130] run blktests nvme/050 at 2024-01-15 13:21:44 [ 262.465068] nvmet: adding nsid 1 to subsystem blktests-subsystem-1 [ 262.520959] nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349. [ 262.525398] nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices. [ 262.528334] nvme nvme1: creating 4 I/O queues. [ 262.531269] nvme nvme1: new ctrl: "blktests-subsystem-1" [ 262.655310] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306 [ 262.657489] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 233, name: kworker/1:3 [ 262.658976] preempt_count: 1, expected: 0 [ 262.660158] RCU nest depth: 0, expected: 0 [ 262.661372] 4 locks held by kworker/1:3/233: [ 262.662526] #0: ffff888135f04538 ((wq_completion)nvmet-wq){+.+.}-{0:0}, at: process_one_work+0x679/0x1260 [ 262.664235] #1: ffff88812524fd90 ((work_completion)(&iod->work)){+.+.}-{0:0}, at: process_one_work+0x6da/0x1260 [ 262.665963] #2: ffff888133caa870 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_execute_pr_report+0x197/0xbd0 [nvmet] [ 262.667622] #3: ffff888136cdd230 (&ns->pr.pr_lock){.+.+}-{2:2}, at: nvmet_execute_pr_report+0x1b4/0xbd0 [nvmet] [ 262.669369] Preemption disabled at: [ 262.669371] [<0000000000000000>] 0x0 [ 262.671748] CPU: 1 PID: 233 Comm: kworker/1:3 Tainted: G W 6.7.0+ #141 [ 262.673226] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014 [ 262.674789] Workqueue: nvmet-wq nvme_loop_execute_work [nvme_loop] [ 262.676142] Call Trace: [ 262.677234] <TASK> [ 262.678241] dump_stack_lvl+0x71/0x90 [ 262.679336] __might_resched+0x3d5/0x5f0 [ 262.680482] ? __pfx___might_resched+0x10/0x10 [ 262.681675] __kmem_cache_alloc_node+0x2ef/0x340 [ 262.682960] ? nvmet_execute_pr_report+0x313/0xbd0 [nvmet] [ 262.684275] ? nvmet_execute_pr_report+0x313/0xbd0 [nvmet] [ 262.685638] __kmalloc+0x50/0x160 [ 262.686779] nvmet_execute_pr_report+0x313/0xbd0 [nvmet] [ 262.688040] ? lock_is_held_type+0xce/0x120 [ 262.689221] process_one_work+0x74c/0x1260 [ 262.690480] ? __pfx_lock_acquire+0x10/0x10 [ 262.691644] ? __pfx_process_one_work+0x10/0x10 [ 262.692893] ? assign_work+0x16c/0x240 [ 262.694063] worker_thread+0x723/0x1300 [ 262.695216] ? lockdep_hardirqs_on+0x7d/0x100 [ 262.696461] ? __kthread_parkme+0xbd/0x1f0 [ 262.697587] ? __pfx_worker_thread+0x10/0x10 [ 262.698812] kthread+0x2f1/0x3d0 [ 262.700000] ? _raw_spin_unlock_irq+0x24/0x50 [ 262.701179] ? __pfx_kthread+0x10/0x10 [ 262.702340] ret_from_fork+0x30/0x70 [ 262.703427] ? __pfx_kthread+0x10/0x10 [ 262.704560] ret_from_fork_asm+0x1b/0x30 [ 262.705718] </TASK> [ 262.891548] nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1" Also please find my comments in line. On Jan 14, 2024 / 17:26, Guixin Liu wrote: > Test the reservation feature, includes register, acquire, release > and report. > > Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> > --- > tests/nvme/050 | 67 ++++++++++++++++++++++++++ > tests/nvme/050.out | 114 +++++++++++++++++++++++++++++++++++++++++++++ > tests/nvme/rc | 3 ++ > 3 files changed, 184 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..a499f66 > --- /dev/null > +++ b/tests/nvme/050 > @@ -0,0 +1,67 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-3.0+ > +# Copyright (C) 2024 Guixin Liu > +# Copyright (C) 2024 Alibaba Group. > +# > +# Test the reservation Nit: looks too short. How about "Test the NVMe reservaction feature"? > +# > +. tests/nvme/rc > + > +DESCRIPTION="test the reservation" Nit: How about "test the reservation feature"? > +QUICK=1 > + > +requires() { > + _nvme_requires > +} > + > +test() { > + echo "Running ${TEST_NAME}" > + > + _setup_nvmet > + > + local nvmedev > + > + _nvmet_target_setup --blkdev file > + > + _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" > + > + nvmedev=$(_find_nvme_dev "${def_subsysnqn}") > + > + echo "Register" > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 Recent nvme-cli, probably since v2.0 in Apr/2022, replaced this --cdw11 option of the "nvme resv-report" command with --eds option. To allow this test case run regardless of the nvme-cli version, I suggest to check "nvme resv-report" command output at the beginning of the test case and see if it contains the string "--eds". Depending on the check result, we can change the option for the "nvme resv-report" commands in this test case. > + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Replace" > + nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Unregister" > + nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Acquire" > + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 > + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Preempt" > + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Release" > + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + > + echo "Clear" > + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 > + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 > + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1 > + > + _nvme_disconnect_subsys "${def_subsysnqn}" > + > + _nvmet_target_cleanup > + > + echo "Test complete" > +} > diff --git a/tests/nvme/050.out b/tests/nvme/050.out > new file mode 100644 > index 0000000..3be417d > --- /dev/null > +++ b/tests/nvme/050.out > @@ -0,0 +1,114 @@ > +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 > + hostid : f1fb429f7f4856b0b351e6b8de349 When I ran the test case on my system, it failed because of hostid mismatch. Actually, we are trying to make the nvme test cases independent from hostid. To not check the hostid lines for this test case, I suggest to add "| grep -v hostid" to the "nvme resv-report" commands. > + > +Replace > +NVME Reservation success > + > +NVME Reservation status: > + > +gen : 2 > +rtype : 0 > +regctl : 1 > +ptpls : 0 > +regctlext[0] : > + cntlid : 1 > + rcsts : 0 > + rkey : 5 > + hostid : f1fb429f7f4856b0b351e6b8de349 > + > +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 > + hostid : f1fb429f7f4856b0b351e6b8de349 > + > +Preempt > +NVME Reservation Acquire success > + > +NVME Reservation status: > + > +gen : 5 > +rtype : 2 > +regctl : 1 > +ptpls : 0 > +regctlext[0] : > + cntlid : 1 > + rcsts : 1 > + rkey : 4 > + hostid : f1fb429f7f4856b0b351e6b8de349 > + > +Release > +NVME Reservation Release success > + > +NVME Reservation status: > + > +gen : 5 > +rtype : 0 > +regctl : 1 > +ptpls : 0 > +regctlext[0] : > + cntlid : 1 > + rcsts : 0 > + rkey : 4 > + hostid : f1fb429f7f4856b0b351e6b8de349 > + > +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 > + hostid : f1fb429f7f4856b0b351e6b8de349 > + > +NVME Reservation Release success > +disconnected 1 controller(s) > +Test complete > diff --git a/tests/nvme/rc b/tests/nvme/rc > index b0ef1ee..8de59e2 100644 > --- a/tests/nvme/rc > +++ b/tests/nvme/rc > @@ -670,6 +670,9 @@ _create_nvmet_ns() { > mkdir "${ns_path}" > printf "%s" "${blkdev}" > "${ns_path}/device_path" > printf "%s" "${uuid}" > "${ns_path}/device_uuid" > + if [[ -f "${ns_path}/resv_enable" ]]; then > + printf 1 > "${ns_path}/resv_enable" > + fi I think this operation is required only for this test case, so resv_enable should not be set for other test cases. So, it is the better to add an optional argument like "--resv_enable" to _nvmet_target_setup() and propagate it to _create_nvmet_subsystem() and _create_nvmet_ns(). It's the better to make it a separated patch. > printf 1 > "${ns_path}/enable" > } > > -- > 2.30.1 (Apple Git-130) > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH blktests] test/nvme/050: test the reservation feature 2024-01-15 5:15 ` Shinichiro Kawasaki @ 2024-01-16 1:57 ` Guixin Liu 0 siblings, 0 replies; 3+ messages in thread From: Guixin Liu @ 2024-01-16 1:57 UTC (permalink / raw) To: Shinichiro Kawasaki Cc: hch@lst.de, sagi@grimberg.me, kch@nvidia.com, chaitanyak@nvidia.com, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org Hi, Shinichiro: Thanks for the patient reply. 在 2024/1/15 13:15, Shinichiro Kawasaki 写道: > Hello Guixin, thanks for the patch. Good to have new tests for new features :) > > I think this test depends on the kernel side patches you posted [1]. So the test > case should be skipped when the kernel does not have the patches. I suggest to > check it using "resv_enable" configfs file. Please refer to nvme/048 which does > similar check and sets SKIP_REASONS. Agree, it will be changed in v2. > > [1] https://lore.kernel.org/linux-nvme/20240114092314.63694-1-kanie@linux.alibaba.com/ > > When I ran the test case on my system using the kernel v6.7 + the [1] patches, > I observed the kernel BUG below. It looks kmalloc() in nvmet_execute_pr_report() > needs care. OK, I will take a look. > > [ 262.402130] run blktests nvme/050 at 2024-01-15 13:21:44 > [ 262.465068] nvmet: adding nsid 1 to subsystem blktests-subsystem-1 > [ 262.520959] nvmet: creating nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uuid:0f01fb42-9f7f-4856-b0b3-51e60b8de349. > [ 262.525398] nvme nvme1: Please enable CONFIG_NVME_MULTIPATH for full support of multi-port devices. > [ 262.528334] nvme nvme1: creating 4 I/O queues. > [ 262.531269] nvme nvme1: new ctrl: "blktests-subsystem-1" > [ 262.655310] BUG: sleeping function called from invalid context at include/linux/sched/mm.h:306 > [ 262.657489] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 233, name: kworker/1:3 > [ 262.658976] preempt_count: 1, expected: 0 > [ 262.660158] RCU nest depth: 0, expected: 0 > [ 262.661372] 4 locks held by kworker/1:3/233: > [ 262.662526] #0: ffff888135f04538 ((wq_completion)nvmet-wq){+.+.}-{0:0}, at: process_one_work+0x679/0x1260 > [ 262.664235] #1: ffff88812524fd90 ((work_completion)(&iod->work)){+.+.}-{0:0}, at: process_one_work+0x6da/0x1260 > [ 262.665963] #2: ffff888133caa870 (&subsys->lock#2){+.+.}-{3:3}, at: nvmet_execute_pr_report+0x197/0xbd0 [nvmet] > [ 262.667622] #3: ffff888136cdd230 (&ns->pr.pr_lock){.+.+}-{2:2}, at: nvmet_execute_pr_report+0x1b4/0xbd0 [nvmet] > [ 262.669369] Preemption disabled at: > [ 262.669371] [<0000000000000000>] 0x0 > [ 262.671748] CPU: 1 PID: 233 Comm: kworker/1:3 Tainted: G W 6.7.0+ #141 > [ 262.673226] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014 > [ 262.674789] Workqueue: nvmet-wq nvme_loop_execute_work [nvme_loop] > [ 262.676142] Call Trace: > [ 262.677234] <TASK> > [ 262.678241] dump_stack_lvl+0x71/0x90 > [ 262.679336] __might_resched+0x3d5/0x5f0 > [ 262.680482] ? __pfx___might_resched+0x10/0x10 > [ 262.681675] __kmem_cache_alloc_node+0x2ef/0x340 > [ 262.682960] ? nvmet_execute_pr_report+0x313/0xbd0 [nvmet] > [ 262.684275] ? nvmet_execute_pr_report+0x313/0xbd0 [nvmet] > [ 262.685638] __kmalloc+0x50/0x160 > [ 262.686779] nvmet_execute_pr_report+0x313/0xbd0 [nvmet] > [ 262.688040] ? lock_is_held_type+0xce/0x120 > [ 262.689221] process_one_work+0x74c/0x1260 > [ 262.690480] ? __pfx_lock_acquire+0x10/0x10 > [ 262.691644] ? __pfx_process_one_work+0x10/0x10 > [ 262.692893] ? assign_work+0x16c/0x240 > [ 262.694063] worker_thread+0x723/0x1300 > [ 262.695216] ? lockdep_hardirqs_on+0x7d/0x100 > [ 262.696461] ? __kthread_parkme+0xbd/0x1f0 > [ 262.697587] ? __pfx_worker_thread+0x10/0x10 > [ 262.698812] kthread+0x2f1/0x3d0 > [ 262.700000] ? _raw_spin_unlock_irq+0x24/0x50 > [ 262.701179] ? __pfx_kthread+0x10/0x10 > [ 262.702340] ret_from_fork+0x30/0x70 > [ 262.703427] ? __pfx_kthread+0x10/0x10 > [ 262.704560] ret_from_fork_asm+0x1b/0x30 > [ 262.705718] </TASK> > [ 262.891548] nvme nvme1: Removing ctrl: NQN "blktests-subsystem-1" > > > Also please find my comments in line. > > On Jan 14, 2024 / 17:26, Guixin Liu wrote: >> Test the reservation feature, includes register, acquire, release >> and report. >> >> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com> >> --- >> tests/nvme/050 | 67 ++++++++++++++++++++++++++ >> tests/nvme/050.out | 114 +++++++++++++++++++++++++++++++++++++++++++++ >> tests/nvme/rc | 3 ++ >> 3 files changed, 184 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..a499f66 >> --- /dev/null >> +++ b/tests/nvme/050 >> @@ -0,0 +1,67 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-3.0+ >> +# Copyright (C) 2024 Guixin Liu >> +# Copyright (C) 2024 Alibaba Group. >> +# >> +# Test the reservation > Nit: looks too short. How about "Test the NVMe reservaction feature"? OK. >> +# >> +. tests/nvme/rc >> + >> +DESCRIPTION="test the reservation" > Nit: How about "test the reservation feature"? > Same sure. >> +QUICK=1 >> + >> +requires() { >> + _nvme_requires >> +} >> + >> +test() { >> + echo "Running ${TEST_NAME}" >> + >> + _setup_nvmet >> + >> + local nvmedev >> + >> + _nvmet_target_setup --blkdev file >> + >> + _nvme_connect_subsys "${nvme_trtype}" "${def_subsysnqn}" >> + >> + nvmedev=$(_find_nvme_dev "${def_subsysnqn}") >> + >> + echo "Register" >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 > Recent nvme-cli, probably since v2.0 in Apr/2022, replaced this --cdw11 option > of the "nvme resv-report" command with --eds option. To allow this test case > run regardless of the nvme-cli version, I suggest to check "nvme resv-report" > command output at the beginning of the test case and see if it contains the > string "--eds". Depending on the check result, we can change the option for the > "nvme resv-report" commands in this test case. My nvme-cli is v1.16, I will update it to the newest version to take a look. >> + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Replace" >> + nvme resv-register "/dev/${nvmedev}n1" --crkey=4 --nrkey=5 --rrega=2 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Unregister" >> + nvme resv-register "/dev/${nvmedev}n1" --crkey=5 --rrega=1 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Acquire" >> + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 >> + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Preempt" >> + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --racqa=1 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Release" >> + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rtype=2 --rrela=0 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + >> + echo "Clear" >> + nvme resv-register "/dev/${nvmedev}n1" --nrkey=4 --rrega=0 >> + nvme resv-acquire "/dev/${nvmedev}n1" --crkey=4 --rtype=1 --racqa=0 >> + nvme resv-report "/dev/${nvmedev}n1" --cdw11=1 >> + nvme resv-release "/dev/${nvmedev}n1" --crkey=4 --rrela=1 >> + >> + _nvme_disconnect_subsys "${def_subsysnqn}" >> + >> + _nvmet_target_cleanup >> + >> + echo "Test complete" >> +} >> diff --git a/tests/nvme/050.out b/tests/nvme/050.out >> new file mode 100644 >> index 0000000..3be417d >> --- /dev/null >> +++ b/tests/nvme/050.out >> @@ -0,0 +1,114 @@ >> +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 >> + hostid : f1fb429f7f4856b0b351e6b8de349 > When I ran the test case on my system, it failed because of hostid mismatch. > Actually, we are trying to make the nvme test cases independent from hostid. To > not check the hostid lines for this test case, I suggest to add > "| grep -v hostid" to the "nvme resv-report" commands. OK, I miss that def_hostid is not same for different system. >> + >> +Replace >> +NVME Reservation success >> + >> +NVME Reservation status: >> + >> +gen : 2 >> +rtype : 0 >> +regctl : 1 >> +ptpls : 0 >> +regctlext[0] : >> + cntlid : 1 >> + rcsts : 0 >> + rkey : 5 >> + hostid : f1fb429f7f4856b0b351e6b8de349 >> + >> +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 >> + hostid : f1fb429f7f4856b0b351e6b8de349 >> + >> +Preempt >> +NVME Reservation Acquire success >> + >> +NVME Reservation status: >> + >> +gen : 5 >> +rtype : 2 >> +regctl : 1 >> +ptpls : 0 >> +regctlext[0] : >> + cntlid : 1 >> + rcsts : 1 >> + rkey : 4 >> + hostid : f1fb429f7f4856b0b351e6b8de349 >> + >> +Release >> +NVME Reservation Release success >> + >> +NVME Reservation status: >> + >> +gen : 5 >> +rtype : 0 >> +regctl : 1 >> +ptpls : 0 >> +regctlext[0] : >> + cntlid : 1 >> + rcsts : 0 >> + rkey : 4 >> + hostid : f1fb429f7f4856b0b351e6b8de349 >> + >> +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 >> + hostid : f1fb429f7f4856b0b351e6b8de349 >> + >> +NVME Reservation Release success >> +disconnected 1 controller(s) >> +Test complete >> diff --git a/tests/nvme/rc b/tests/nvme/rc >> index b0ef1ee..8de59e2 100644 >> --- a/tests/nvme/rc >> +++ b/tests/nvme/rc >> @@ -670,6 +670,9 @@ _create_nvmet_ns() { >> mkdir "${ns_path}" >> printf "%s" "${blkdev}" > "${ns_path}/device_path" >> printf "%s" "${uuid}" > "${ns_path}/device_uuid" >> + if [[ -f "${ns_path}/resv_enable" ]]; then >> + printf 1 > "${ns_path}/resv_enable" >> + fi > I think this operation is required only for this test case, so resv_enable > should not be set for other test cases. So, it is the better to add an optional > argument like "--resv_enable" to _nvmet_target_setup() and propagate it to > _create_nvmet_subsystem() and _create_nvmet_ns(). It's the better to make > it a separated patch. Yeah, sure, it will be changes in v2. >> printf 1 > "${ns_path}/enable" >> } >> >> -- >> 2.30.1 (Apple Git-130) >> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-01-16 1:57 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-14 9:26 [PATCH blktests] test/nvme/050: test the reservation feature Guixin Liu 2024-01-15 5:15 ` Shinichiro Kawasaki 2024-01-16 1:57 ` Guixin Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox