* [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