Re: [PATCH v3 05/13] generic/1226: Add atomic write test using fio crc check verifier

John Garry
2025-07-17T13:00:32Z

On 12/07/2025 15:12, Ojaswin Mujoo wrote:
> From: "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
>
> This adds atomic write test using fio based on it's crc check verifier.
> fio adds a crc for each data block. If the underlying device supports atomic
> write then it is guaranteed that we will never have a mix data from two
> threads writing on the same physical block.

I think that you should mention that 2-phase approach.

Is there something which ensures that we have fio which supports
RWF_ATOMIC? fio for some time supported the "atomic" cmdline param, but
did not do anything until recently

>
> Co-developed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
> ---
> tests/generic/1226 | 101 +++++++++++++++++++++++++++++++++++++++++
> tests/generic/1226.out | 2 +

Was this tested with xfs?

> 2 files changed, 103 insertions(+)
> create mode 100755 tests/generic/1226
> create mode 100644 tests/generic/1226.out
>
> diff --git a/tests/generic/1226 b/tests/generic/1226
> new file mode 100755
> index 00000000..455fc55f
> --- /dev/null
> +++ b/tests/generic/1226
> @@ -0,0 +1,101 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 IBM Corporation. All Rights Reserved.
> +#
> +# FS QA Test 1226
> +#
> +# Validate FS atomic write using fio crc check verifier.
> +#
> +. ./common/preamble
> +. ./common/atomicwrites
> +
> +_begin_fstest auto aio rw atomicwrites
> +
> +_require_scratch_write_atomic
> +_require_odirect
> +_require_aio
> +
> +_scratch_mkfs >> $seqres.full 2>&1
> +_scratch_mount
> +
> +touch "$SCRATCH_MNT/f1"
> +awu_min_write=$(_get_atomic_write_unit_min "$SCRATCH_MNT/f1")
> +awu_max_write=$(_get_atomic_write_unit_max "$SCRATCH_MNT/f1")
> +blocksize=$(_max "$awu_min_write" "$((awu_max_write/2))")
> +
> +fio_config=$tmp.fio
> +fio_out=$tmp.fio.out
> +
> +FIO_LOAD=$(($(nproc) * 2 * LOAD_FACTOR))
> +SIZE=$((100 * 1024 * 1024))
> +
> +function create_fio_configs()
> +{
> + create_fio_aw_config

it's strange ordering in this file, since create_fio_aw_config is
declared below here

> + create_fio_verify_config

same

> +}
> +
> +function create_fio_verify_config()
> +{
> +cat >$fio_verify_config <<EOF
> + [verify-job]
> + direct=1
> + ioengine=libaio
> + rw=randwrite

is this really required? Maybe it is. I would use read if something was
required for this param

> + bs=$blocksize
> + fallocate=native
> + filename=$SCRATCH_MNT/test-file
> + size=$SIZE
> + iodepth=$FIO_LOAD
> + group_reporting=1
> +
> + verify_only=1
> + verify=crc32c
> + verify_fatal=1
> + verify_state_save=0
> + verify_write_sequence=0
> +EOF
> +}
> +
> +function create_fio_aw_config()
> +{
> +cat >$fio_aw_config <<EOF
> + [atomicwrite-job]
> + direct=1
> + ioengine=libaio
> + rw=randwrite
> + bs=$blocksize
> + fallocate=native
> + filename=$SCRATCH_MNT/test-file
> + size=$SIZE
> + iodepth=$FIO_LOAD
> + numjobs=$FIO_LOAD
> + group_reporting=1
> + atomic=1
> +
> + verify_state_save=0
> + verify=crc32c
> + do_verify=0
> +EOF
> +}
> +
> +fio_aw_config=$tmp.aw.fio
> +fio_verify_config=$tmp.verify.fio
> +
> +create_fio_configs
> +_require_fio $fio_aw_config
> +
> +cat $fio_aw_config >> $seqres.full
> +cat $fio_verify_config >> $seqres.full
> +
> +$FIO_PROG $fio_aw_config >> $seqres.full
> +ret1=$?
> +$FIO_PROG $fio_verify_config >> $seqres.full
> +ret2=$?
> +
> +[[ $ret1 -eq 0 && $ret2 -eq 0 ]] || _fail "fio with atomic write failed"
> +
> +# success, all done
> +echo Silence is golden
> +status=0
> +exit
> diff --git a/tests/generic/1226.out b/tests/generic/1226.out
> new file mode 100644
> index 00000000..6dce0ea5
> --- /dev/null
> +++ b/tests/generic/1226.out
> @@ -0,0 +1,2 @@
> +QA output created by 1226
> +Silence is golden