On 2023/11/19 21:14, Zorro Lang wrote:
On Sat, Nov 18, 2023 at 11:08:10AM +0800, Baokun Li wrote:
On 2023/11/17 22:24, Zorro Lang wrote:
Hi Baokun,
Thanks for this new coverage, some review points as below ...
Hi Zorro,
Thank you for your careful review！
+
+# Initalize a 4k 100M ext4 fs
+dev_size=$((100 * 1024 * 1024))
+MKFS_OPTIONS="-b 4096 $MKFS_OPTIONS" _scratch_mkfs_sized $dev_size \
+ >>$seqres.full 2>&1 || _fail "mkfs failed"
Can we use bsize=$(_get_file_block_size ....) to get the real block size,
to avoid use a specific mkfs option?
If so, my second question is if this case can be a generic case. The test
steps look common enough, I'm glad to see what will happen on other
filesystems through this test.
The reason for specifying 4096 for formatting here is that mkfs.ext4
specifies a
default block size of 1024 when the disk image is 100M.
Is the reproducer related with the block size. If it's not, we can use
"bsize=$(_get_file_block_size $SCRATCH_MNT)" to get the block size, and change
later 4096 to $bsize.
This problem is really hard to reproduce when the block size is 1024,
but the
good thing is that the default block size specified in
_scratch_mkfs_sized()
is 4096, so we can leave the block size of 4096 unspecified here.
Although these operations appear to be generic scenarios, the reproduction
of
this problem relies on ext4's pre-allocation mechanism. Block allocation
requests
are scaled up in ext4_mb_normalize_request(), and the extra blocks are used
for
preallocation to reduce fragmentation. Therefore, this test case is placed
only in
the ext4 test suite.
Sure it's a reproducer for ext4, but it can be run for other filesystems
which support falloc and finsert. And:
_require_xfs_io_command "falloc"
_require_xfs_io_command "finsert"
will make sure the current FSTYP supports these two features. So it can
be a bug coverage for ext4, and a generic test case for other fs.
Totally agree, I'll put it in generic test suite in next version, the
execution of
this use case takes only 1-2s, it doesn't seem to have any impact.
+
+_scratch_mount
+
+# Reserve 1M space
+$XFS_IO_PROG -f -c "falloc 0 1M" "${SCRATCH_MNT}/tmp" >> $seqres.full 2>&1
+
+# Create a file with logical block numbers close to overflow
+$XFS_IO_PROG -f -c "falloc 0 10M" "${SCRATCH_MNT}/file" >> $seqres.full 2>&1
+$XFS_IO_PROG -f -c "finsert 1M 16777203M" "${SCRATCH_MNT}/file" >> $seqres.full 2>&1
_require_xfs_io_command "finsert"
Okay.
+
+# Filling up the free space ensures that the pre-allocated space is the reserved space.
+$XFS_IO_PROG -f -c "falloc 0 80388096" "${SCRATCH_MNT}/fill" >> $seqres.full 2>&1
Can we make sure this step fill the whole free space? There's a helper in
common/populate named _fill_fs, please check if it's what you want?
The remaining space for a 100M 4k ext4 image after the previous operations
is
80388096, so we can confirm that we can fill the entire free space here.
Of course, using _fill_fs is also workable, but it's faster to do it in a
single fallocate.
I'm not sure how the reservation works, if there're 90M free space, and you
try to reserve 90+M, will once fallocate calling trys to reserve each free
blocks, before returning error? If it's not, I think the _fill_fs might be
what you want.
Thanks,
Zorro
ext4_fallocate will try to allocate all free blocks if the size to be
allocated exceeds
the free space on the filesystem, but will still return an error. I've
tested _fill_fs and
it works well too. I will use _fill_fs in the next release to make this
use case more generic.
Thanks!
--
With Best Regards,
Baokun Li
.
