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

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

On Aug 07, 2026 / 09:32, John Garry wrote:
> 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.

FYI, I applied the patch in the current form.

> 
> 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...

Thanks, it sounds a good idea to me. I guess we can use bash "eval" feature. I
guess the helper function like below will do the trick:

	_wait_event_timeout() {
		local timeout_in_seconds=${1}
		local condition_str=${2}
		local i

		for ((i = 0; i < timeout_in_seconds; i++)); do
			if eval "$condition_str"; then
				return 0
			fi
			sleep 1
		done
		return 1
	}

With this, _check_nvme_core_ref_count() can be reimplemented as follows:

	nvme_core_refcnt_is_orig() {
		local refcnt_orig=${1}
		local refcnt=$(_module_use_count nvme_core)

		[[ "$refcnt" == "" || "$refcnt" -eq "$refcnt_orig" ]]
	}

	_check_nvme_core_ref_count() {
		_wait_event_timeout 10 "nvme_core_refcnt_is_orig $refcnt_orig"
	}

This looks a bit simpler.