Re: [PATCH blktests v2] nvme/068: check module reference count with patience

[Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

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