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 <[email protected]>

Just one nit below, otherwise looks good.

Reviewed-by: Vishal Verma <[email protected]>

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' ],

Reply via email to