On Mon, Sep 08, 2025 at 03:27:57PM +0100, John Garry wrote: > On 05/09/2025 18:06, Ojaswin Mujoo wrote: > > On Tue, Sep 02, 2025 at 04:49:26PM +0100, John Garry wrote: > > > On 22/08/2025 09:02, Ojaswin Mujoo wrote: > > > > --- > > > > tests/generic/1230 | 397 +++++++++++++++++++++++++++++++++++++++++ > > > > tests/generic/1230.out | 2 + > > > > 2 files changed, 399 insertions(+) > > > > create mode 100755 tests/generic/1230 > > > > create mode 100644 tests/generic/1230.out > > > > <...> > > > > + > > > > + bytes_written=$(tail -n 1 $tmp.aw | cut -d" " -f4) > > > > + echo "# Bytes written in 0.2s: $bytes_written" >> $seqres.full > > > > + > > > > + filesize=$((bytes_written * 3)) > > > > + echo "# Setting \$filesize=$filesize" >> $seqres.full > > > > + > > > > + rm $tmp.aw > > > > + sleep 0.5 > > > > + > > > > + _scratch_cycle_mount > > > > + > > > > +} > > > > + > > > > +create_mixed_mappings() { > > > > > > Is this same as patch 08/12? > > > > I believe you mean the [D]SYNC tests, yes it is the same. > > then maybe factor out the test, if possible. I assume that this sort of > approach is taken for xfstests. > I'm not sure what you mean by factor out the *test*. We are testing different things there and the only thing common in the tests is creation of mixed mapping files and the check to ensure we didn't tear data. In case you mean to factor out the create_mixed_mappings() helper into common/rc, sure I can do that but I'm unsure if at this point it would be very useful for other tests. > > > > > > > > > + local file=$1 > > > > + local size_bytes=$2 > > > > + > > > > + echo "# Filling file $file with alternate mappings till size $size_bytes" >> $seqres.full > > > > + #Fill the file with alternate written and unwritten blocks > > > > + local off=0 > > > > + local operations=("W" "U") > > > > + > > > > + for ((i=0; i<$((size_bytes / blksz )); i++)); do > > > > + index=$(($i % ${#operations[@]})) > > > > + map="${operations[$index]}" > > > > + <...> > > > > +# Loop 20 times to shake out any races due to shutdown > > > > +for ((iter=0; iter<20; iter++)) > > > > +do > > > > + echo >> $seqres.full > > > > + echo "------ Iteration $iter ------" >> $seqres.full > > > > + > > > > + echo >> $seqres.full > > > > + echo "# Starting data integrity test for atomic writes over mixed mapping" >> $seqres.full > > > > + test_data_integrity_mixed > > > > + > > > > + echo >> $seqres.full > > > > + echo "# Starting data integrity test for atomic writes over fully written mapping" >> $seqres.full > > > > + test_data_integrity_writ > > > > + > > > > + echo >> $seqres.full > > > > + echo "# Starting data integrity test for atomic writes over fully unwritten mapping" >> $seqres.full > > > > + test_data_integrity_unwrit > > > > + > > > > + echo >> $seqres.full > > > > + echo "# Starting data integrity test for atomic writes over holes" >> $seqres.full > > > > + test_data_integrity_hole > > > > + > > > > + echo >> $seqres.full > > > > + echo "# Starting filesize integrity test for atomic writes" >> $seqres.full > > > > > > what does "Starting filesize integrity test" mean? > > > > Basically other tests already truncate the file to a higher value and > > then perform the shut down test. Here we actually do append atomic > > writes since we want to also stress the i_size update paths during > > shutdown to ensure that doesn't cause any tearing with atomic writes. > > > > I can maybe rename it to: > > > > > > echo "# Starting data integrity test for atomic append writes" >> $seqres.full > > > > Thanks for the review! > > > > It's just the name "integrity" that throws me a bit.. So I mean integrity as in writes are not tearing after the shutdown. That's how we have worded the other sub-tests above. Regards, ojaswin