Re: [ndctl PATCH] test/btt-stress.sh: add stress test for BTT lane race

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Fri, 2026-04-24 at 16:36 -0700, Alison Schofield wrote:
> The btt-check unit test exposed data mismatches during BTT I/O in a
> CI environment, indicating a race in lane acquisition that can lead
> to silent data corruption. The failure was not reliably reproduced
> under typical test conditions.
> 
> Add a targeted stress test that repeatedly writes, reads, and verifies
> data on a BTT namespace while background readers contend for BTT lanes
> and CPU loops increase preemption pressure.
> 
> The test reproduces the race on an unfixed kernel and passes with the
> lane ownership fix applied.
> 
> Assisted-By: Claude Sonnet 4.5
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Just one nit below, otherwise looks good.

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

Also, side note, looks like this ran for ~45 seconds on my laptop making
it the second longest running test in ndctl - maybe that's fine. Also
all the 'while :; do :;' processes pegged half my CPUs to 100% for the
entire time, but maybe that's also fine :) - I see that's intentional of
course.

> ---
>  test/btt-stress.sh | 111 +++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build   |   2 +
>  2 files changed, 113 insertions(+)
>  create mode 100755 test/btt-stress.sh
> 
> diff --git a/test/btt-stress.sh b/test/btt-stress.sh
> new file mode 100755
> index 000000000000..c1892f536d75
> --- /dev/null
> +++ b/test/btt-stress.sh
> @@ -0,0 +1,111 @@
> +#!/bin/bash -E
> +# SPDX-License-Identifier: GPL-2.0
> +
> +dev=""
> +mode=""
> +size=""
> +sector_size=""
> +blockdev=""
> +rc=77
> +
> +. $(dirname $0)/common
> +
> +trap 'err $LINENO' ERR
> +
> +check_min_kver "7.1" || do_skip "known BTT lane race before fix"
> +
> +# Stress BTT I/O under contention to exercise lane acquisition races.
> +# Background readers contend for lanes while CPU loops increase
> +# preemption pressure.
> +
> +create() {

Usually for other scripts in ndctl, the convention has been to be more
C-like and put the opening { on the next line.

> +	json=$($NDCTL create-namespace -b "$NFIT_TEST_BUS0" -t pmem -m sector)
> +	rc=2
> +	eval "$(echo "$json" | json2var)"

I was going to complain about the eval and the json2var 'sed' parsing to
get stuff out of json, but I see this has been around since forever and
used in a bunch of other places.

Ideally this should just be multiple jq -r <filter> invocations, to
extract each variable - more wordy, but avoids the eval.. But this can
be a future treewide cleanup.

> +	[ -n "$dev" ] || err "$LINENO"
> +	[ "$mode" = "sector" ] || err "$LINENO"
> +	[ -n "$size" ] || err "$LINENO"
> +	[ -n "$sector_size" ] || err "$LINENO"
> +	[ -n "$blockdev" ] || err "$LINENO"
> +	[ "$size" -gt 0 ] || err "$LINENO"
> +}
> +
> +# Start background workers:
> +#   - readers contend for lanes
> +#   - CPU loops increase preemption
> +start_bg_workers() {
> +	local ncpus
> +	ncpus=$(nproc)
> +	local nworkers=$((ncpus / 2))
> +
> +	# Ensure at least one worker, cap to limit runtime noise
> +	[ $nworkers -lt 1 ] && nworkers=1
> +	[ $nworkers -gt 8 ] && nworkers=8
> +
> +	BG_PIDS=()
> +	local i
> +	for i in $(seq 1 $nworkers); do
> +		# Reader: contends for lanes (use O_DIRECT to avoid page cache)
> +		(while :; do
> +			dd if=/dev/"$blockdev" of=/dev/null \
> +				bs="$sector_size" count=256 \
> +				iflag=direct >/dev/null 2>&1 || true
> +		done) &
> +		BG_PIDS+=($!)
> +
> +		# CPU hog: increase preemption
> +		(while :; do :; done) &
> +		BG_PIDS+=($!)
> +	done
> +	echo "started $nworkers readers + $nworkers CPU hogs"
> +}
> +
> +stop_bg_workers() {
> +	local pid
> +	for pid in "${BG_PIDS[@]}"; do
> +		kill "$pid" 2>/dev/null || true
> +	done
> +	wait "${BG_PIDS[@]}" 2>/dev/null || true
> +	BG_PIDS=()
> +}
> +
> +# Write, read, and verify data
> +do_io_verify() {
> +	dd if=/dev/urandom of=test-bin \
> +		bs="$sector_size" count=$((size / sector_size)) >/dev/null 2>&1
> +	dd if=test-bin of=/dev/"$blockdev" \
> +		bs="$sector_size" count=$((size / sector_size)) >/dev/null 2>&1
> +	dd if=/dev/"$blockdev" of=test-bin-read \
> +		bs="$sector_size" count=$((size / sector_size)) >/dev/null 2>&1
> +	diff test-bin test-bin-read
> +	rm -f test-bin*
> +}
> +
> +# Run verification under contention
> +test_io_stress() {
> +	local iterations=${1:-20}
> +	echo "=== ${FUNCNAME[0]} ($iterations iterations) ==="
> +
> +	start_bg_workers
> +	trap 'stop_bg_workers; err $LINENO' ERR
> +
> +	local i
> +	for i in $(seq 1 "$iterations"); do
> +		echo "--- iteration $i/$iterations ---"
> +		do_io_verify
> +	done
> +
> +	stop_bg_workers
> +	trap 'err $LINENO' ERR
> +}
> +
> +modprobe nfit_test
> +rc=1
> +reset && create
> +
> +# 30 iterations balances runtime and reproduction probability
> +test_io_stress 30
> +
> +reset
> +_cleanup
> +exit 0
> diff --git a/test/meson.build b/test/meson.build
> index e0e2193bfd51..ee6a18762a17 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -150,6 +150,7 @@ sector_mode = find_program('sector-mode.sh')
>  inject_error = find_program('inject-error.sh')
>  btt_errors = find_program('btt-errors.sh')
>  btt_pad_compat = find_program('btt-pad-compat.sh')
> +btt_stress = find_program('btt-stress.sh')
>  firmware_update = find_program('firmware-update.sh')
>  rescan_partitions = find_program('rescan-partitions.sh')
>  inject_smart = find_program('inject-smart.sh')
> @@ -185,6 +186,7 @@ tests = [
>    [ 'sector-mode.sh',         sector_mode,        'ndctl' ],
>    [ 'inject-error.sh',        inject_error,	  'ndctl' ],
>    [ 'btt-errors.sh',          btt_errors,	  'ndctl' ],
> +  [ 'btt-stress.sh',          btt_stress,	  'ndctl' ],
>    [ 'hugetlb',                hugetlb,		  'ndctl' ],
>    [ 'btt-pad-compat.sh',      btt_pad_compat,	  'ndctl' ],
>    [ 'ack-shutdown-count-set', ack_shutdown_count, 'ndctl' ],