> > + > > +for (( BSIZE = 1024; BSIZE <= 4096; BSIZE *= 2 )); do > > + > > + length=$(($BLOCKS * $BSIZE)) > > + case $FSTYP in > > + xfs) > > + _scratch_mkfs -b size=$BSIZE >> $seqres.full 2>&1 > > + ;; > > + ext4) > > + _scratch_mkfs -b $BSIZE >> $seqres.full 2>&1 > > + ;; > > + esac > > I think we typically try to define things like blocksize in the test > configuration. The MKFS_OPTIONS env. var. can be defined with the > correct options for the associated fs being tested. That eliminates the > need to loop and for the fs-specific _scratch_mkfs calls above. Okay, I will change. > > > + _scratch_mount >> $seqres.full 2>&1 > > + > > + # Write file > > + $XFS_IO_PROG -f -c "pwrite 0 $length" -c fsync $testfile > /dev/null > > + > > + > > + # Insert alternate blocks > > Trailing space in the above comment. > > > + for (( j=0; j < $(($BLOCKS/2)); j++ )); do > > + offset=$((($j*$BSIZE)*2)) > > + $XFS_IO_PROG -c "finsert $offset $BSIZE" $testfile > /dev/null > > + done > > + > > + # Check if 100 extents are present > > + $XFS_IO_PROG -c "fiemap -v" $testfile | grep "^ *[0-9]*:" |wc -l > > + > > + _check_scratch_fs > > + if [ $? -ne 0 ]; then > > + status=1 > > + exit > > + fi > > + > > + # Collapse alternate blocks > > ... and here as well. Okay, I wil fix both points. > > > + for (( j=0; j < $(($BLOCKS/2)); j++ )); do > > + offset=$((($j*$BSIZE))) > > + $XFS_IO_PROG -c "fcollapse $offset $BSIZE" $testfile > /dev/null > > + done > > + > > + # Check if 1 extents are present > > + $XFS_IO_PROG -c "fiemap -v" $testfile | grep "^ *[0-9]*:" |wc -l > > + > > + _check_scratch_fs > > + if [ $? -ne 0 ]; then > > + status=1 > > + exit > > + fi > > + > > Maybe create a copy of the original file before we run through the > insert and collapse sequence and cmp the files here as a last step? Good idea. I will add your point. Thanks for your review! > > Brian -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html