[PATCH v4 0/2] blktests: use patient module remover

[PATCH v4 0/2] blktests: use patient module remover

[Luis Chamberlain]

This rebases the patient module remover series onto the current blktests
tree and addresses all the feedback from v3 from a long time ago [0]. This
came up recently as Swarna noticed the same flaky bug and I got reminded
to re-send a new iteraiton.

We now have the modprobe --wait upstream so use that if avaiable.

The patient module remover addresses race conditions where module removal
can fail due to userspace temporarily bumping the refcount (e.g., via
blkdev_open() calls). If your version of kmod supports modprobe --wait,
we use that. Otherwise we implement our own patient module remover.

[0] https://lore.kernel.org/all/20221220235324.1445248-2-mcgrof@kernel.org/T/#u

Changes in v4:
  - Rebased onto current blktests master
  - Fixed built-in module handling: _patient_rmmod() now checks for
    the existence of /sys/module/$module_sys/refcnt to detect built-in
    modules and returns success early (Shinichiro Kawasaki)
  - Fixed timeout parameter timing issue: MODPROBE_PATIENT_RM_TIMEOUT_SECONDS
    is now evaluated dynamically inside _patient_rmmod() rather than at
    source time, allowing tests to override the timeout value
    (Shinichiro Kawasaki)
  - Preserved SKIP_REASONS logic in _init_null_blk() when modprobe fails,
    so tests requiring loadable modules are properly skipped rather than
    failing (Shinichiro Kawasaki)
  - Fixed typo in comment: "modprobe -p" -> "modprobe --wait"
    (Luis Chamberlain)
  - Fixed path reference bug: used $module_sys instead of $module for
    /sys/module path checks (Luis Chamberlain)
  - Combined nested while conditions for cleaner code style
    (Shinichiro Kawasaki)
  - Removed redundant $module printing in error messages
    (Shinichiro Kawasaki)
  - Updated to use --wait=TIMEOUT_MSEC syntax (with =) to match the
    upstream kmod implementation


Luis Chamberlain (2):
  blktests: replace module removal with patient module removal
  tests/srp/rc: replace module removal with patient module removal

 common/multipath-over-rdma |  11 +---
 common/null_blk            |   6 +-
 common/nvme                |   9 +--
 common/rc                  | 126 +++++++++++++++++++++++++++++++++++++
 common/scsi_debug          |  14 ++---
 tests/srp/rc               |  19 ++----
 6 files changed, 148 insertions(+), 37 deletions(-)

-- 
2.51.0

[PATCH v4 1/2] blktests: replace module removal with patient module removal

[Luis Chamberlain]

A long time ago, in a galaxy far, far away...

I ran into some odd scsi_debug false positives with fstests. This
prompted me to look into them given these false positives prevents
me from moving forward with establishing a test baseline with high
number of cycles. That is, this stupid issue was prevening creating
high confidence in testing.

I reported it [0] and exchanged some ideas with Doug. However, in
the end, despite efforts to help things with scsi_debug there were
still issues lingering which seemed to defy our expectations upstream.
One of the last hanging fruit issues is and always has been that
userspace expectations for proper module removal has been broken,
so in the end I have demonstrated this is a generic issue [1].

Long ago a WAIT option for module removal was added... that was then
removed as it was deemed not needed as folks couldn't figure out when
these races happened. The races are actually pretty easy to trigger, it
was just never properly documented. A simpe blkdev_open() will easily
bump a module refcnt, and these days many thing scan do that sort of
thing.

The proper solution is to implement then a patient module removal
on kmod and that has been merged now as modprobe --wait=MSEC option.
We need a work around to open code a similar solution for users of
old versions of kmod. An open coded solution for fstests exists
there for over a year now. This now provides the respective blktests
implementation.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=212337
[1] https://bugzilla.kernel.org/show_bug.cgi?id=214015

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Rebased-by: Claude AI
---
 common/multipath-over-rdma |  11 +---
 common/null_blk            |   6 +-
 common/nvme                |   9 +--
 common/rc                  | 126 +++++++++++++++++++++++++++++++++++++
 common/scsi_debug          |  14 ++---
 tests/srp/rc               |   4 +-
 6 files changed, 145 insertions(+), 25 deletions(-)

diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index 1084f80..d082700 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -4,6 +4,7 @@
 #
 # Functions and global variables used by the srp tests.
 
+. common/rc
 . common/shellcheck
 . common/null_blk
 
@@ -427,14 +428,8 @@ stop_soft_rdma() {
 		      echo "$i ..."
 		      rdma link del "${i}" || echo "Failed to remove ${i}"
 		done
-	if ! _unload_module rdma_rxe 10; then
-		echo "Unloading rdma_rxe failed"
-		return 1
-	fi
-	if ! _unload_module siw 10; then
-		echo "Unloading siw failed"
-		return 1
-	fi
+	_patient_rmmod rdma_rxe || return 1
+	_patient_rmmod siw  || return 1
 	} >>"$FULL"
 }
 
diff --git a/common/null_blk b/common/null_blk
index bbb6f78..f5fccc1 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -5,6 +5,7 @@
 # null_blk helper functions.
 
 . common/shellcheck
+. common/rc
 
 _have_null_blk() {
 	_have_driver null_blk
@@ -38,7 +39,8 @@ _init_null_blk() {
 	local args=("$@")
 	if (( RUN_FOR_ZONED )); then args+=("zoned=1"); fi
 
-	if ! modprobe -r null_blk || ! modprobe null_blk "${args[@]}" ; then
+	_patient_rmmod null_blk || return 1
+	if ! modprobe null_blk "${args[@]}"; then
 		SKIP_REASONS+=("requires modular null_blk")
 		return 1
 	fi
@@ -79,5 +81,5 @@ _configure_null_blk() {
 _exit_null_blk() {
 	_remove_null_blk_devices
 	udevadm settle
-	modprobe -r -q null_blk
+	_patient_rmmod null_blk
 }
diff --git a/common/nvme b/common/nvme
index 3d43790..049850e 100644
--- a/common/nvme
+++ b/common/nvme
@@ -4,6 +4,7 @@
 # nvme helper functions.
 
 . common/shellcheck
+. common/rc
 
 def_traddr="127.0.0.1"
 def_adrfam="ipv4"
@@ -210,13 +211,13 @@ _cleanup_nvmet() {
 
 	if [[ "${nvme_trtype}" == "fc" ]]; then
 		_nvme_fcloop_del_lport "${def_local_wwnn}" "${def_local_wwpn}"
-		modprobe -rq nvme-fcloop 2>/dev/null
+		_patient_rmmod nvme-fcloop
 	fi
-	modprobe -rq nvme-"${nvme_trtype}" 2>/dev/null
+	_patient_rmmod nvme-"${nvme_trtype}"
 	if [[ "${nvme_trtype}" != "loop" ]]; then
-		modprobe -rq nvmet-"${nvme_trtype}" 2>/dev/null
+		_patient_rmmod nvmet-"${nvme_trtype}"
 	fi
-	modprobe -rq nvmet 2>/dev/null
+	_patient_rmmod nvmet
 	if [[ "${nvme_trtype}" == "rdma" ]]; then
 		stop_soft_rdma
 	fi
diff --git a/common/rc b/common/rc
index ea92970..556581f 100644
--- a/common/rc
+++ b/common/rc
@@ -736,3 +736,129 @@ _min() {
 	done
 	echo "$ret"
 }
+
+_has_modprobe_patient()
+{
+	modprobe --help >& /dev/null || return 1
+	modprobe --help | grep -q "\-\-wait" || return 1
+	return 0
+}
+
+# Check whether modprobe --wait is supported and set up the patient module
+# remover command. This is evaluated at source time, so we need to handle
+# the timeout dynamically in _patient_rmmod() for cases where tests want
+# to override it.
+MODPROBE_HAS_WAIT=""
+if _has_modprobe_patient; then
+	MODPROBE_HAS_WAIT="yes"
+fi
+
+if [[ -z "$MODPROBE_PATIENT_RM_TIMEOUT_SECONDS" ]]; then
+	export MODPROBE_PATIENT_RM_TIMEOUT_SECONDS="50"
+fi
+
+# checks the refcount and returns 0 if we can safely remove the module. rmmod
+# does this check for us, but we can use this to also iterate checking for this
+# refcount before we even try to remove the module. This is useful when using
+# debug test modules which take a while to quiesce.
+_patient_rmmod_check_refcnt()
+{
+	local module=$1
+	local refcnt=0
+
+	refcnt=$(cat "/sys/module/$module/refcnt" 2>/dev/null)
+	if [[ $? -ne 0 || $refcnt -eq 0 ]]; then
+		return 0
+	fi
+	return 1
+}
+
+# Tries to wait patiently to remove a module by ensuring first
+# the refcnt is 0 and then trying to remove the module over and over
+# again within the time allowed. The timeout is configurable per test, just set
+# MODPROBE_PATIENT_RM_TIMEOUT_SECONDS prior to calling this function.
+# This applies to both cases where kmod supports the patient module remover
+# (modprobe --wait) and where it does not.
+#
+# If your version of kmod supports modprobe --wait, we use that instead.
+# Otherwise we have to implement a patient module remover ourselves.
+_patient_rmmod()
+{
+	local module=$1
+	local max_tries_max=$MODPROBE_PATIENT_RM_TIMEOUT_SECONDS
+	local max_tries=0
+	local mod_ret=0
+	local refcnt_is_zero=0
+	# Since we are looking for a directory we must adopt the
+	# specific directory used by scripts/Makefile.lib for
+	# KBUILD_MODNAME
+	local module_sys=${module//-/_}
+
+	# Check if module is built-in or not loaded
+	if [[ ! -d "/sys/module/$module_sys" ]]; then
+		return 0
+	fi
+
+	# Check if this is a built-in module (no refcnt file means built-in)
+	if [[ ! -f "/sys/module/$module_sys/refcnt" ]]; then
+		return 0
+	fi
+
+	if [[ -n $MODPROBE_HAS_WAIT ]]; then
+		local timeout_ms=$((MODPROBE_PATIENT_RM_TIMEOUT_SECONDS * 1000))
+		modprobe -r --wait="${timeout_ms}" "$module"
+		mod_ret=$?
+		if [[ $mod_ret -ne 0 ]]; then
+			echo "kmod patient module removal for $module timed out waiting for refcnt to become 0 using timeout of $max_tries_max returned $mod_ret"
+		fi
+		return $mod_ret
+	fi
+
+	max_tries=$max_tries_max
+
+	while [[ "$max_tries" != "0" ]]; do
+		if _patient_rmmod_check_refcnt "$module_sys"; then
+			refcnt_is_zero=1
+			break
+		fi
+		sleep 1
+		((max_tries--))
+	done
+
+	if [[ $refcnt_is_zero -ne 1 ]]; then
+		echo "custom patient module removal for $module timed out waiting for refcnt to become 0 using timeout of $max_tries_max"
+		return 1
+	fi
+
+	# If we ran out of time but our refcnt check confirms we had
+	# a refcnt of 0, just try to remove the module once.
+	if [[ "$max_tries" == "0" ]]; then
+		modprobe -r "$module"
+		return $?
+	fi
+
+	# If we have extra time left. Use the time left to now try to
+	# persistently remove the module. We do this because although through
+	# the above we found refcnt to be 0, removal can still fail since
+	# userspace can always race to bump the refcnt. An example is any
+	# blkdev_open() calls against a block device. These issues have been
+	# tracked and documented in the following bug reports, which justifies
+	# our need to do this in userspace:
+	# https://bugzilla.kernel.org/show_bug.cgi?id=212337
+	# https://bugzilla.kernel.org/show_bug.cgi?id=214015
+	while [[ $max_tries != 0 ]] && [[ -d /sys/module/$module_sys ]]; do
+		modprobe -r "$module" 2> /dev/null
+		mod_ret=$?
+		if [[ $mod_ret == 0 ]]; then
+			break;
+		fi
+		sleep 1
+		((max_tries--))
+	done
+
+	if [[ $mod_ret -ne 0 ]]; then
+		echo "custom patient module removal for $module timed out trying to remove using timeout of $max_tries_max last try returned $mod_ret"
+	fi
+
+	return $mod_ret
+}
diff --git a/common/scsi_debug b/common/scsi_debug
index 89c4801..5f40aa4 100644
--- a/common/scsi_debug
+++ b/common/scsi_debug
@@ -4,6 +4,8 @@
 #
 # scsi_debug helper functions.
 
+. common/rc
+
 _have_scsi_debug() {
 	_have_driver scsi_debug
 }
@@ -98,14 +100,8 @@ _init_scsi_debug() {
 		args+=(zbc=host-managed zone_nr_conv=0)
 	fi
 
-	if ! _unload_module scsi_debug 10; then
-		echo "Unloading scsi_debug failed" >&2
-		return 1
-	fi
-	if ! modprobe scsi_debug "${args[@]}"; then
-		echo "Loading scsi_debug ${args[*]} failed" >&2
-		return 1
-	fi
+	_patient_rmmod scsi_debug || return 1
+	modprobe scsi_debug "${args[@]}" || return 1
 
 	udevadm settle
 
@@ -180,7 +176,7 @@ _exit_scsi_debug() {
 	udevadm settle
 
 	if _module_file_exists scsi_debug; then
-		_unload_module scsi_debug 10
+		_patient_rmmod scsi_debug
 		return
 	fi
 
diff --git a/tests/srp/rc b/tests/srp/rc
index 47b9546..8585272 100755
--- a/tests/srp/rc
+++ b/tests/srp/rc
@@ -502,7 +502,7 @@ start_lio_srpt() {
 	if modinfo ib_srpt | grep -q '^parm:[[:blank:]]*rdma_cm_port:'; then
 		opts+=("rdma_cm_port=${srp_rdma_cm_port}")
 	fi
-	_unload_module ib_srpt
+	_patient_rmmod ib_srpt
 	modprobe ib_srpt "${opts[@]}" || return $?
 	i=0
 	for r in "${vdev_path[@]}"; do
@@ -564,7 +564,7 @@ stop_lio_srpt() {
 			 target_core_file target_core_stgt target_core_user \
 			 target_core_mod
 	do
-		_unload_module $m 10 || return $?
+		_patient_rmmod $m || return $?
 	done
 }
 
-- 
2.51.0

[PATCH v4 2/2] tests/srp/rc: replace module removal with patient module removal

[Luis Chamberlain]

Bart had put the remove_mpath_devs() call inside a loop because multipathd
keeps running while the loop is ongoing and hence can modify paths
while the loop is running. The races that multipathd can trigger with the
module refcnt is precisely the sort of races which patient module
removal is supposed to address.

Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Rebased-by: Claude AI
---
 tests/srp/rc | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/tests/srp/rc b/tests/srp/rc
index 8585272..2d3d615 100755
--- a/tests/srp/rc
+++ b/tests/srp/rc
@@ -331,19 +331,10 @@ start_srp_ini() {
 
 # Unload the SRP initiator driver.
 stop_srp_ini() {
-	local i
-
 	log_out
-	for ((i=40;i>=0;i--)); do
-		remove_mpath_devs || return $?
-		_unload_module ib_srp >/dev/null 2>&1 && break
-		sleep 1
-	done
-	if [ -e /sys/module/ib_srp ]; then
-		echo "Error: unloading kernel module ib_srp failed"
-		return 1
-	fi
-	_unload_module scsi_transport_srp || return $?
+	remove_mpath_devs || return $?
+	_patient_rmmod ib_srp || return 1
+	_patient_rmmod scsi_transport_srp || return $?
 }
 
 # Associate the LIO device with name $1/$2 with file $3 and SCSI serial $4.
-- 
2.51.0

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Bart Van Assche]

On 11/26/25 9:11 AM, Luis Chamberlain wrote:
> Long ago a WAIT option for module removal was added... that was then
> removed as it was deemed not needed as folks couldn't figure out when
> these races happened. The races are actually pretty easy to trigger, it
> was just never properly documented. A simpe blkdev_open() will easily
> bump a module refcnt, and these days many thing scan do that sort of
> thing.

It would be appreciated if "thing" could be replaced with a more
specific description of what actually happens.
> The proper solution is to implement then a patient module removal
> on kmod and that has been merged now as modprobe --wait=MSEC option.
> We need a work around to open code a similar solution for users of
> old versions of kmod. An open coded solution for fstests exists
> there for over a year now. This now provides the respective blktests
> implementation.

How can it be concluded what the proper solution is without explaining
the root cause first? Please add an explanation of the root cause. I
assume that the root cause is that some references are dropped
asynchronously after module removal has been requested for the first
time?
> +_has_modprobe_patient()
> +{
> +	modprobe --help >& /dev/null || return 1
> +	modprobe --help | grep -q "\-\-wait" || return 1
> +	return 0
> +}

Please combine the above two modprobe invocations into a single
invocation and leave out the superfluous return statements, e.g. as
follows:

	modprobe --help |& grep -q -- --wait

Additionally, wouldn't has_modprobe_wait be a better name for this
function?

> +# Check whether modprobe --wait is supported and set up the patient module
> +# remover command. This is evaluated at source time, so we need to handle

remover -> removal?

> +# checks the refcount and returns 0 if we can safely remove the module. rmmod
> +# does this check for us, but we can use this to also iterate checking for this
> +# refcount before we even try to remove the module. This is useful when using
> +# debug test modules which take a while to quiesce.
> +_patient_rmmod_check_refcnt()
> +{
> +	local module=$1
> +	local refcnt=0
> +
> +	refcnt=$(cat "/sys/module/$module/refcnt" 2>/dev/null)
> +	if [[ $? -ne 0 || $refcnt -eq 0 ]]; then
> +		return 0
> +	fi
> +	return 1
> +}

The refcnt initializer can be removed safely and the return statements
can be removed too.

> +	# Check if module is built-in or not loaded
> +	if [[ ! -d "/sys/module/$module_sys" ]]; then
> +		return 0
> +	fi

Is the comment above the if-statement correct? /sys/module/${module_sys}
is also created for built-in kernel drivers that set the .owner field,
isn't it? See also the sysfs_create_link(... "module") calls in
drivers/base/.

> +	max_tries=$max_tries_max
> +
> +	while [[ "$max_tries" != "0" ]]; do
> +		if _patient_rmmod_check_refcnt "$module_sys"; then
> +			refcnt_is_zero=1
> +			break
> +		fi
> +		sleep 1
> +		((max_tries--))
> +	done

Has it been considered to use a for-loop for the above code? E.g.
something like

   for ((max_tries=max_tries_max; max_tries != 0; max_tries--)); do
     ...
   done

Thanks,

Bart.

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Shinichiro Kawasaki]

On 11/27/25 2:11 AM, Luis Chamberlain wrote:
> A long time ago, in a galaxy far, far away...
> 
> I ran into some odd scsi_debug false positives with fstests. This
> prompted me to look into them given these false positives prevents
> me from moving forward with establishing a test baseline with high
> number of cycles. That is, this stupid issue was prevening creating
> high confidence in testing.
> 
> I reported it [0] and exchanged some ideas with Doug. However, in
> the end, despite efforts to help things with scsi_debug there were
> still issues lingering which seemed to defy our expectations upstream.
> One of the last hanging fruit issues is and always has been that
> userspace expectations for proper module removal has been broken,
> so in the end I have demonstrated this is a generic issue [1].
> 
> Long ago a WAIT option for module removal was added... that was then
> removed as it was deemed not needed as folks couldn't figure out when
> these races happened. The races are actually pretty easy to trigger, it
> was just never properly documented. A simpe blkdev_open() will easily
> bump a module refcnt, and these days many thing scan do that sort of
> thing.
> 
> The proper solution is to implement then a patient module removal
> on kmod and that has been merged now as modprobe --wait=MSEC option.
> We need a work around to open code a similar solution for users of
> old versions of kmod. An open coded solution for fstests exists
> there for over a year now. This now provides the respective blktests
> implementation.
> 
> [0] https://bugzilla.kernel.org/show_bug.cgi?id=212337
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=214015

Luis, thanks for the v4 series and sorry for this slow response. Please 
find my comments in line.

> 
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> Rebased-by: Claude AI

This "Rebased-by" tag is interesting. But it does not look widely used 
and may not be so valuable to leave in the commit log.

> ---
>   common/multipath-over-rdma |  11 +---
>   common/null_blk            |   6 +-
>   common/nvme                |   9 +--
>   common/rc                  | 126 +++++++++++++++++++++++++++++++++++++
>   common/scsi_debug          |  14 ++---
>   tests/srp/rc               |   4 +-
>   6 files changed, 145 insertions(+), 25 deletions(-)
> 
> diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
> index 1084f80..d082700 100644
> --- a/common/multipath-over-rdma
> +++ b/common/multipath-over-rdma
> @@ -4,6 +4,7 @@
>   #
>   # Functions and global variables used by the srp tests.
>   
> +. common/rc

This commit created a number of dependencies between common/rc and other 
common/X. The dependency added to common/nvme triggered a shellcheck 
warning SC2178. This warning itself is not due to this commit, 
(actually, it is shellcheck bug), but now I think this shows less 
dependency is the better.

I suggest to add the new helper functions to not common/rc but the 
"check" script. This avoids the new dependencies. It also provide the 
chance to refactor _unload_module() in the "check" script.

>   . common/shellcheck
>   . common/null_blk

[...]

> diff --git a/common/rc b/common/rc
> index ea92970..556581f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -736,3 +736,129 @@ _min() {
>   	done
>   	echo "$ret"
>   }
> +
> +_has_modprobe_patient()
> +{
> +	modprobe --help >& /dev/null || return 1
> +	modprobe --help | grep -q "\-\-wait" || return 1

During test runs, the line above prints out the error message,

    "grep: warning: stray \ before -"

repeatedly. The suggested change by Bart in his response will avoid the 
message.

> +	return 0
> +}

Including this part, I tried to move this common/rc changes into the 
"check" script, and did a few trial runs. It looks working good.

I'm now revising this series by myself to repost as v5 to reflect may 
comments as well as the comments by Bart. In case you want to respin 
this by yourself, please let me know. Thanks!

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Luis Chamberlain]

On Thu, Dec 18, 2025 at 10:44:13AM +0000, Shinichiro Kawasaki wrote:
> I'm now revising this series by myself to repost as v5 to reflect may 
> comments as well as the comments by Bart.

Oh not at all, this series just collects dust in my queue so I get to it
when I can, which is never lately so please feel free to take it on.

 Luis

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Shinichiro Kawasaki]

On 11/27/25 2:45 AM, Bart Van Assche wrote:
> On 11/26/25 9:11 AM, Luis Chamberlain wrote:
>> Long ago a WAIT option for module removal was added... that was then
>> removed as it was deemed not needed as folks couldn't figure out when
>> these races happened. The races are actually pretty easy to trigger, it
>> was just never properly documented. A simpe blkdev_open() will easily
>> bump a module refcnt, and these days many thing scan do that sort of
>> thing.
> 
> It would be appreciated if "thing" could be replaced with a more
> specific description of what actually happens.
>> The proper solution is to implement then a patient module removal
>> on kmod and that has been merged now as modprobe --wait=MSEC option.
>> We need a work around to open code a similar solution for users of
>> old versions of kmod. An open coded solution for fstests exists
>> there for over a year now. This now provides the respective blktests
>> implementation.
> 
> How can it be concluded what the proper solution is without explaining
> the root cause first? Please add an explanation of the root cause. I
> assume that the root cause is that some references are dropped
> asynchronously after module removal has been requested for the first
> time?

I agree that the clarification on the above points will improve the commit
message. Said that, I do not think lack of the clarifications should delay the
application of this series.

Swarna, Luis, if there is any URL to the "flaky bug" that Swaran faced, please
share. I can add it as a Link tag to show another reason for this series.

[...]

>> +	# Check if module is built-in or not loaded
>> +	if [[ ! -d "/sys/module/$module_sys" ]]; then
>> +		return 0
>> +	fi
> 
> Is the comment above the if-statement correct? /sys/module/${module_sys}
> is also created for built-in kernel drivers that set the .owner field,
> isn't it? See also the sysfs_create_link(... "module") calls in
> drivers/base/.

As far as I understand, there is a corner case that the /sys/module/X
directories are not created for built-in kernel drivers, such as nvme-fc or
nvmet-fc. When the driver does not have any parameter, the directory is not
created, probably.

Other than these, I plan to reflect the comments by Bart to the v5 series that
I'm preparing now. Thanks.

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Swarna Prabhu]

On Fri, Dec 19, 2025 at 05:29:06AM +0000, Shinichiro Kawasaki wrote:
> On 11/27/25 2:45 AM, Bart Van Assche wrote:
> > On 11/26/25 9:11 AM, Luis Chamberlain wrote:
> >> Long ago a WAIT option for module removal was added... that was then
> >> removed as it was deemed not needed as folks couldn't figure out when
> >> these races happened. The races are actually pretty easy to trigger, it
> >> was just never properly documented. A simpe blkdev_open() will easily
> >> bump a module refcnt, and these days many thing scan do that sort of
> >> thing.
> > 
> > It would be appreciated if "thing" could be replaced with a more
> > specific description of what actually happens.
> >> The proper solution is to implement then a patient module removal
> >> on kmod and that has been merged now as modprobe --wait=MSEC option.
> >> We need a work around to open code a similar solution for users of
> >> old versions of kmod. An open coded solution for fstests exists
> >> there for over a year now. This now provides the respective blktests
> >> implementation.
> > 
> > How can it be concluded what the proper solution is without explaining
> > the root cause first? Please add an explanation of the root cause. I
> > assume that the root cause is that some references are dropped
> > asynchronously after module removal has been requested for the first
> > time?
> 
> I agree that the clarification on the above points will improve the commit
> message. Said that, I do not think lack of the clarifications should delay the
> application of this series.
> 
> Swarna, Luis, if there is any URL to the "flaky bug" that Swaran faced, please
> share. I can add it as a Link tag to show another reason for this series.
>

I donot have any URL to the bug, but below is the description for it: 

I had seen a sporadic kernel hang  while running block tests (block/003) when testing  
scsi driver with sector size 16k which is detailed in the  testing description of the 
cover letter sent for RFC v1 series for scsi fix. 
(link here- https://lore.kernel.org/all/20251202021522.188419-1-sw.prabhu6@gmail.com/ ).
The fio task triggerd at test block/003 would race with udevs generated while stressing 
scsi debug module in test - block/001.

I havent been able to reproduce the hang in the current setup lately, so i pasted below the 
snippet of the dmesg i had captured when the hang occured with block/003 test:

55.240584]  __mutex_lock.constprop.0+0x3fc/0xa60
[10755.241693]  ? _raw_spin_unlock+0x15/0x30
[10755.242630]  bdev_open+0x2ac/0x3d0
[10755.243446]  ? __pfx_blkdev_open+0x10/0x10
[10755.244399]  blkdev_open+0xc6/0x120
[10755.245217]  do_dentry_open+0x242/0x430
[10755.246128]  vfs_open+0x2a/0xe0                                                                                                                         
[10755.246876]  path_openat+0x31e/0x12e0
[10755.247752]  ? getname_flags.part.0+0x26/0x1c0
[10755.248777]  do_filp_open+0xbf/0x170
[10755.249637]  ? _raw_spin_unlock_irqrestore+0x23/0x40
[10755.250778]  ? __create_object+0x5e/0x90
[10755.251695]  ? preempt_count_add+0x47/0xa0
[10755.252645]  ? __virt_addr_valid+0xf5/0x170
[10755.253636]  ? __check_object_size+0x249/0x2d0
[10755.254670]  ? alloc_fd+0x11a/0x180
[10755.255502]  do_sys_openat2+0x70/0xd0
[10755.256360]  __x64_sys_openat+0x69/0xa0
[10755.257276]  do_syscall_64+0x50/0xb60
[10755.258138]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[10755.259294] RIP: 0033:0x7f22ceca49ee
[10755.260141] RSP: 002b:00007ffc0a8b18e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
[10755.261864] RAX: ffffffffffffffda RBX: 00007f22cc568f00 RCX: 00007f22ceca49ee
[10755.263489] RDX: 0000000000040002 RSI: 00007f22cb30d2b0 RDI: ffffffffffffff9c
[10755.265102] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[10755.266727] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000040002
[10755.268304] R13: 00007f22c3efa000 R14: 0000000000000000 R15: 000055b79b6f020a
[10755.269724]  </TASK>
[10755.270209] INFO: task fio:109969 is blocked on a mutex likely owned by task systemd-udevd:404.
[10755.271920] task:systemd-udevd   state:D stack:0     pid:404   tgid:404   ppid:1      task_flags:0x400100 flags:0x00080003
[10755.273863] Call Trace:
[10755.274324]  <TASK>
[10755.274729]  __schedule+0x54c/0xc10
[10755.275372]  ? _raw_spin_unlock_irqrestore+0x23/0x40
[10755.276249]  schedule+0x26/0xd0
[10755.276782]  schedule_timeout+0x71/0xf0
[10755.277432]  ? __pfx_process_timeout+0x10/0x10
[10755.278167]  io_schedule_timeout+0x4d/0x70
[10755.278845]  __wait_for_common+0x99/0x1b0
[10755.279517]  ? __pfx_io_schedule_timeout+0x10/0x10
[10755.280281]  blk_execute_rq+0xeb/0x160
[10755.280857]  scsi_execute_cmd+0x11e/0x430 [scsi_mod]
[10755.281653]  ? disk_scan_partitions+0x5d/0xf0
[10755.282319]  ? __x64_sys_ioctl+0x92/0xe0
[10755.282914]  sd_spinup_disk+0x102/0x4b0 [sd_mod]
[10755.283624]  ? _raw_spin_unlock_irqrestore+0x23/0x40
[10755.284341]  sd_revalidate_disk+0xda/0x2710 [sd_mod]	


Hope this helps.

Thanks
Swarna

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Shinichiro Kawasaki]

On Dec 22, 2025 / 18:11, Swarna Prabhu wrote:
> On Fri, Dec 19, 2025 at 05:29:06AM +0000, Shinichiro Kawasaki wrote:
> > On 11/27/25 2:45 AM, Bart Van Assche wrote:
> > > On 11/26/25 9:11 AM, Luis Chamberlain wrote:
> > >> Long ago a WAIT option for module removal was added... that was then
> > >> removed as it was deemed not needed as folks couldn't figure out when
> > >> these races happened. The races are actually pretty easy to trigger, it
> > >> was just never properly documented. A simpe blkdev_open() will easily
> > >> bump a module refcnt, and these days many thing scan do that sort of
> > >> thing.
> > > 
> > > It would be appreciated if "thing" could be replaced with a more
> > > specific description of what actually happens.
> > >> The proper solution is to implement then a patient module removal
> > >> on kmod and that has been merged now as modprobe --wait=MSEC option.
> > >> We need a work around to open code a similar solution for users of
> > >> old versions of kmod. An open coded solution for fstests exists
> > >> there for over a year now. This now provides the respective blktests
> > >> implementation.
> > > 
> > > How can it be concluded what the proper solution is without explaining
> > > the root cause first? Please add an explanation of the root cause. I
> > > assume that the root cause is that some references are dropped
> > > asynchronously after module removal has been requested for the first
> > > time?
> > 
> > I agree that the clarification on the above points will improve the commit
> > message. Said that, I do not think lack of the clarifications should delay the
> > application of this series.
> > 
> > Swarna, Luis, if there is any URL to the "flaky bug" that Swaran faced, please
> > share. I can add it as a Link tag to show another reason for this series.
> >
> 
> I donot have any URL to the bug, but below is the description for it: 
> 
> I had seen a sporadic kernel hang  while running block tests (block/003) when testing  
> scsi driver with sector size 16k which is detailed in the  testing description of the 
> cover letter sent for RFC v1 series for scsi fix. 
> (link here- https://lore.kernel.org/all/20251202021522.188419-1-sw.prabhu6@gmail.com/ ).
> The fio task triggerd at test block/003 would race with udevs generated while stressing 
> scsi debug module in test - block/001.
> 
> I havent been able to reproduce the hang in the current setup lately, so i pasted below the 
> snippet of the dmesg i had captured when the hang occured with block/003 test:
> 

Swarna, thanks for the description. The problem you faced looks like the same
problem that Luis described. I will amend the commit log to add the link to this
e-mail thread for recording.

As Luis described, the problem looks having multiple aspects, and this patient
module removal solution addresses one of the aspects. Another aspect I noticed
was that lots of udev events influence following test cases. Similar problem was
observed for loop/010 [*], and solution for loop/010 was disabling udev events
during the testcase run. At this moment, I think and hope this patient module
removal solution is enough for block/001, and there is no need to disable udev
events in block/001, probably.

[*] https://github.com/linux-blktests/blktests/issues/181

Re: [PATCH v4 1/2] blktests: replace module removal with patient module removal

[Shinichiro Kawasaki]

On Nov 26, 2025 / 09:11, Luis Chamberlain wrote:
[...]
> diff --git a/common/rc b/common/rc
> index ea92970..556581f 100644
> --- a/common/rc
> +++ b/common/rc
[...]
> +# Tries to wait patiently to remove a module by ensuring first
> +# the refcnt is 0 and then trying to remove the module over and over
> +# again within the time allowed. The timeout is configurable per test, just set
> +# MODPROBE_PATIENT_RM_TIMEOUT_SECONDS prior to calling this function.
> +# This applies to both cases where kmod supports the patient module remover
> +# (modprobe --wait) and where it does not.
> +#
> +# If your version of kmod supports modprobe --wait, we use that instead.
> +# Otherwise we have to implement a patient module remover ourselves.
> +_patient_rmmod()
> +{
> +	local module=$1
> +	local max_tries_max=$MODPROBE_PATIENT_RM_TIMEOUT_SECONDS
> +	local max_tries=0
> +	local mod_ret=0
> +	local refcnt_is_zero=0
> +	# Since we are looking for a directory we must adopt the
> +	# specific directory used by scripts/Makefile.lib for
> +	# KBUILD_MODNAME
> +	local module_sys=${module//-/_}
> +
> +	# Check if module is built-in or not loaded
> +	if [[ ! -d "/sys/module/$module_sys" ]]; then
> +		return 0
> +	fi
> +
> +	# Check if this is a built-in module (no refcnt file means built-in)
> +	if [[ ! -f "/sys/module/$module_sys/refcnt" ]]; then
> +		return 0
> +	fi
> +
> +	if [[ -n $MODPROBE_HAS_WAIT ]]; then
> +		local timeout_ms=$((MODPROBE_PATIENT_RM_TIMEOUT_SECONDS * 1000))
> +		modprobe -r --wait="${timeout_ms}" "$module"

While I evaluated this patch, I noticed that the modprobe command above can
return zero status code, even when it fails to remove the target module. I will
add a follow-up patch to the v5 series to check the reference count even when
the command above success to catch such cases.

Also, I will replace the short option -r with the long option --remove for
script readability and stability.

> +		mod_ret=$?
> +		if [[ $mod_ret -ne 0 ]]; then
> +			echo "kmod patient module removal for $module timed out waiting for refcnt to become 0 using timeout of $max_tries_max returned $mod_ret"
> +		fi
> +		return $mod_ret
> +	fi