Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH blktests v2] nvme/068: check module reference count with patience
@ 2026-08-06  3:31 Shin'ichiro Kawasaki
  2026-08-06  7:24 ` John Garry
  0 siblings, 1 reply; 4+ messages in thread
From: Shin'ichiro Kawasaki @ 2026-08-06  3:31 UTC (permalink / raw)
  To: linux-nvme; +Cc: John Garry, Shin'ichiro Kawasaki

The test case nvme/068 fails depending on the system it runs on with
the failure message below:

   nvme/068 (tr=rdma) (NVMe multipath delayed removal test)     [failed]
       runtime    ...  11.357s
       --- tests/nvme/068.out    2026-08-03 17:24:32.394566266 +0000
       +++ /home/runner/blktests/results/nodev_tr_rdma/nvme/068.out.bad  2026-08-03 18:15:17.461243816 +0000
       @@ -1,3 +1,4 @@
        Running nvme/068
        pwrite: Input/output error
       +module refcount not as original
        Test complete

The test case checks the reference count of the nvme_core module. If the
count is not the expected value, it fails with the error message.
However, when the test system is slow, it takes some time until the
reference count gets updated. When the check is too early, the count
mismatch happens. To avoid the unexpected failure, repeat the reference
count check.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
I ran the test case nvme/068 with the patch using the kernel that does
not include the commit below:

  3f150f0f010f ("nvme-multipath: put module reference when delayed removal work is canceled")

And I observed the test case failed. When the kernel includes the commit,
the test case passes. This indicates the patch does not break the
reference count sanity check capability of the test case.

Changes from v1:
- Per review comments, handled empty "$refcnt" case and explicit 0 return

 tests/nvme/068 | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/nvme/068 b/tests/nvme/068
index 4aee95b..cab57cd 100755
--- a/tests/nvme/068
+++ b/tests/nvme/068
@@ -27,6 +27,19 @@ _delayed_nvme_reconnect_ctrl() {
 	_nvme_connect_subsys
 }
 
+_check_nvme_core_ref_count() {
+	local refcnt i
+
+	for ((i = 0; i < 10; i++)); do
+		refcnt=$(_module_use_count nvme_core)
+		if [ "$refcnt" == "" ] || [ "$refcnt" -eq "$refcnt_orig" ]; then
+			return 0
+		fi
+		sleep 1
+	done
+	return 1
+}
+
 test() {
 	echo "Running ${TEST_NAME}"
 
@@ -98,8 +111,7 @@ test() {
 	if ! _nvmf_wait_for_state "${def_subsysnqn}" "live" ; then
 		echo "subsystem did not return"
 	fi
-	refcnt=$(_module_use_count nvme_core)
-	if [ "$refcnt" != "" ] && [ "$refcnt" -ne "$refcnt_orig" ]; then
+	if ! _check_nvme_core_ref_count; then
 		echo "module refcount not as original"
 	fi
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] nvme/068: check module reference count with patience
  2026-08-06  3:31 [PATCH blktests v2] nvme/068: check module reference count with patience Shin'ichiro Kawasaki
@ 2026-08-06  7:24 ` John Garry
  2026-08-06 11:30   ` Shin'ichiro Kawasaki
  0 siblings, 1 reply; 4+ messages in thread
From: John Garry @ 2026-08-06  7:24 UTC (permalink / raw)
  To: Shin'ichiro Kawasaki, linux-nvme

On 06/08/2026 04:31, Shin'ichiro Kawasaki wrote:
> The test case nvme/068 fails depending on the system it runs on with the failure
> message below: nvme/068 (tr=rdma) (NVMe multipath delayed removal test) [failed]
> runtime .. . 11. 357s --- tests/nvme/068. out 2026-08-03 17: 24: 32. 394566266 +0000
> 
> 
> The test case nvme/068 fails depending on the system it runs on with
> the failure message below:
> 
>      nvme/068 (tr=rdma) (NVMe multipath delayed removal test)     [failed]
>          runtime    ...  11.357s
>          --- tests/nvme/068.out    2026-08-03 17:24:32.394566266 +0000
>          +++ /home/runner/blktests/results/nodev_tr_rdma/nvme/068.out.bad  2026-08-03 18:15:17.461243816 +0000
>          @@ -1,3 +1,4 @@
>           Running nvme/068
>           pwrite: Input/output error
>          +module refcount not as original
>           Test complete
> 
> The test case checks the reference count of the nvme_core module. If the
> count is not the expected value, it fails with the error message.
> However, when the test system is slow, it takes some time until the
> reference count gets updated. When the check is too early, the count
> mismatch happens. To avoid the unexpected failure, repeat the reference
> count check.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Reviewed-by: John Garry <john.g.garry@oracle.com>

Just one comment/query below...

> ---
> I ran the test case nvme/068 with the patch using the kernel that does
> not include the commit below:
> 
>     3f150f0f010f ("nvme-multipath: put module reference when delayed removal work is canceled")
> 
> And I observed the test case failed. When the kernel includes the commit,
> the test case passes. This indicates the patch does not break the
> reference count sanity check capability of the test case.
> 
> Changes from v1:
> - Per review comments, handled empty "$refcnt" case and explicit 0 return
> 
>    tests/nvme/068 | 16 ++++++++++++++--
>    1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/nvme/068 b/tests/nvme/068
> index 4aee95b..cab57cd 100755
> --- a/tests/nvme/068
> +++ b/tests/nvme/068
> @@ -27,6 +27,19 @@ _delayed_nvme_reconnect_ctrl() {
>    	_nvme_connect_subsys
>    }
>    
> +_check_nvme_core_ref_count() {
> +	local refcnt i
> +
> +	for ((i = 0; i < 10; i++)); do
> +		refcnt=$(_module_use_count nvme_core)
> +		if [ "$refcnt" == "" ] || [ "$refcnt" -eq "$refcnt_orig" ]; then

I thought that refcnt_orig was local to test(), so I am unsure how it is 
accessible in this function...but it seems to work.

> +			return 0
> +		fi
> +		sleep 1
> +	done
> +	return 1
> +}
> +
>    test() {
>    	echo "Running ${TEST_NAME}"
>    
> @@ -98,8 +111,7 @@ test() {
>    	if ! _nvmf_wait_for_state "${def_subsysnqn}" "live" ; then
>    		echo "subsystem did not return"
>    	fi
> -	refcnt=$(_module_use_count nvme_core)
> -	if [ "$refcnt" != "" ] && [ "$refcnt" -ne "$refcnt_orig" ]; then
> +	if ! _check_nvme_core_ref_count; then
>    		echo "module refcount not as original"
>    	fi
>    
> -- 
> 2.54.0
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] nvme/068: check module reference count with patience
  2026-08-06  7:24 ` John Garry
@ 2026-08-06 11:30   ` Shin'ichiro Kawasaki
  2026-08-07  8:32     ` John Garry
  0 siblings, 1 reply; 4+ messages in thread
From: Shin'ichiro Kawasaki @ 2026-08-06 11:30 UTC (permalink / raw)
  To: John Garry; +Cc: linux-nvme

On Aug 06, 2026 / 08:24, John Garry wrote:
> On 06/08/2026 04:31, Shin'ichiro Kawasaki wrote:
> > The test case nvme/068 fails depending on the system it runs on with the failure
> > message below: nvme/068 (tr=rdma) (NVMe multipath delayed removal test) [failed]
> > runtime .. . 11. 357s --- tests/nvme/068. out 2026-08-03 17: 24: 32. 394566266 +0000
> > 
> > 
> > The test case nvme/068 fails depending on the system it runs on with
> > the failure message below:
> > 
> >      nvme/068 (tr=rdma) (NVMe multipath delayed removal test)     [failed]
> >          runtime    ...  11.357s
> >          --- tests/nvme/068.out    2026-08-03 17:24:32.394566266 +0000
> >          +++ /home/runner/blktests/results/nodev_tr_rdma/nvme/068.out.bad  2026-08-03 18:15:17.461243816 +0000
> >          @@ -1,3 +1,4 @@
> >           Running nvme/068
> >           pwrite: Input/output error
> >          +module refcount not as original
> >           Test complete
> > 
> > The test case checks the reference count of the nvme_core module. If the
> > count is not the expected value, it fails with the error message.
> > However, when the test system is slow, it takes some time until the
> > reference count gets updated. When the check is too early, the count
> > mismatch happens. To avoid the unexpected failure, repeat the reference
> > count check.
> > 
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> 
> Reviewed-by: John Garry <john.g.garry@oracle.com>

Thanks!

> 
> Just one comment/query below...
> 
> > ---
> > I ran the test case nvme/068 with the patch using the kernel that does
> > not include the commit below:
> > 
> >     3f150f0f010f ("nvme-multipath: put module reference when delayed removal work is canceled")
> > 
> > And I observed the test case failed. When the kernel includes the commit,
> > the test case passes. This indicates the patch does not break the
> > reference count sanity check capability of the test case.
> > 
> > Changes from v1:
> > - Per review comments, handled empty "$refcnt" case and explicit 0 return
> > 
> >    tests/nvme/068 | 16 ++++++++++++++--
> >    1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/nvme/068 b/tests/nvme/068
> > index 4aee95b..cab57cd 100755
> > --- a/tests/nvme/068
> > +++ b/tests/nvme/068
> > @@ -27,6 +27,19 @@ _delayed_nvme_reconnect_ctrl() {
> >    	_nvme_connect_subsys
> >    }
> > +_check_nvme_core_ref_count() {
> > +	local refcnt i
> > +
> > +	for ((i = 0; i < 10; i++)); do
> > +		refcnt=$(_module_use_count nvme_core)
> > +		if [ "$refcnt" == "" ] || [ "$refcnt" -eq "$refcnt_orig" ]; then
> 
> I thought that refcnt_orig was local to test(), so I am unsure how it is
> accessible in this function...but it seems to work.

This is a bash uniqueness. Here I quote a relevant paragraph from the Bash
manual [*]. Some paragraphs follow and explain how bash handles local variable
scope.

  Variables local to the function are declared with the local builtin (local
  variables). Ordinarily, variables and their values are shared between a
  function and its caller. These variables are visible only to the function and
  the commands it invokes. This is particularly important when a shell function
  calls other functions.

refcnt_orig was declared as a local variable by the caller of
_check_nvme_core_ref_count(), then it is visible in _check_nvme_core_ref_count()
also. I used this feature intentionally, but I understand it looks weird. If you
like, I will update the patch to pass refcnt_orig as a function argument
instead.

[*] https://www.gnu.org/software/bash/manual/bash.html#Shell-Functions


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH blktests v2] nvme/068: check module reference count with patience
  2026-08-06 11:30   ` Shin'ichiro Kawasaki
@ 2026-08-07  8:32     ` John Garry
  0 siblings, 0 replies; 4+ messages in thread
From: John Garry @ 2026-08-07  8:32 UTC (permalink / raw)
  To: Shin'ichiro Kawasaki, John Garry; +Cc: linux-nvme

On 8/6/26 12:30, Shin'ichiro Kawasaki wrote:
>>> +_check_nvme_core_ref_count() {
>>> +	local refcnt i
>>> +
>>> +	for ((i = 0; i < 10; i++)); do
>>> +		refcnt=$(_module_use_count nvme_core)
>>> +		if [ "$refcnt" == "" ] || [ "$refcnt" -eq "$refcnt_orig" ]; then
>> I thought that refcnt_orig was local to test(), so I am unsure how it is
>> accessible in this function...but it seems to work.
> This is a bash uniqueness. Here I quote a relevant paragraph from the Bash
> manual [*]. Some paragraphs follow and explain how bash handles local variable
> scope.
> 
>    Variables local to the function are declared with the local builtin (local
>    variables). Ordinarily, variables and their values are shared between a
>    function and its caller. These variables are visible only to the function and
>    the commands it invokes. This is particularly important when a shell function
>    calls other functions.
> 
> refcnt_orig was declared as a local variable by the caller of
> _check_nvme_core_ref_count(), then it is visible in _check_nvme_core_ref_count()
> also. I used this feature intentionally, but I understand it looks weird. If you
> like, I will update the patch to pass refcnt_orig as a function argument
> instead.
> 
> [*]https://www.gnu.org/software/bash/manual/bash.html#Shell-Functions

Understood, thanks for the info.

BTW, on another topic, it seems to be a common pattern to loop waiting 
for a condition to be true in the blktests codebase, like:

+_check_nvme_core_ref_count() {
+	local refcnt i
+
+	for ((i = 0; i < 10; i++)); do
+		refcnt=$(_module_use_count nvme_core)
+		if [ "$refcnt" == "" ] || [ "$refcnt" -eq "$refcnt_orig" ]; then
+			return 0
+		fi
+		sleep 1
+	done
+	return 1
+}

In the kernel, we have functions like wait_event_timeout(wq_head, 
condition, timeout), which calls @condition and checks the result to 
break the loop and determine success. Could it be possible to have such 
a helper in blktests? I don't know how...


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-07  8:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06  3:31 [PATCH blktests v2] nvme/068: check module reference count with patience Shin'ichiro Kawasaki
2026-08-06  7:24 ` John Garry
2026-08-06 11:30   ` Shin'ichiro Kawasaki
2026-08-07  8:32     ` John Garry

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox