Re: [xfstests PATCH v2] generic: add a partial pages zeroing out test

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Wed, Jan 08, 2025 at 08:40:32PM +0800, Zhang Yi wrote:
> On 2025/1/8 17:54, Ojaswin Mujoo wrote:
> > On Wed, Dec 25, 2024 at 08:51:20PM +0800, Zhang Yi wrote:
> >> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
> >>
> >> This addresses a data corruption issue encountered during partial page
> >> zeroing in ext4 which the block size is smaller than the page size [1].
> >> Add a new test which is expanded upon generic/567, this test performs a
> >> zeroing range test that spans two partial pages to cover this case, and
> >> also generalize it to work for non-4k page sizes.
> >>
> >> Link: https://lore.kernel.org/linux-ext4/20241220011637.1157197-2-yi.zhang@xxxxxxxxxxxxxxx/ [1]
> >> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
> >> ---
> >> v1->v2:
> >>  - Add a new test instead of modifying generic/567.
> >>  - Generalize the test to work for non-4k page sizes.
> >> v1: https://lore.kernel.org/fstests/20241223023930.2328634-1-yi.zhang@xxxxxxxxxxxxxxx/
> >>
> >>  tests/generic/758     | 76 +++++++++++++++++++++++++++++++++++++++++++
> >>  tests/generic/758.out |  3 ++
> >>  2 files changed, 79 insertions(+)
> >>  create mode 100755 tests/generic/758
> >>  create mode 100644 tests/generic/758.out
> >>
> >> diff --git a/tests/generic/758 b/tests/generic/758
> >> new file mode 100755
> >> index 00000000..e03b5e80
> >> --- /dev/null
> >> +++ b/tests/generic/758
> >> @@ -0,0 +1,76 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (c) 2024 Huawei.  All Rights Reserved.
> >> +#
> >> +# FS QA Test No. generic/758
> >> +#
> >> +# Test mapped writes against zero-range to ensure we get the data
> >> +# correctly written. This can expose data corruption bugs on filesystems
> >> +# where the block size is smaller than the page size.
> >> +#
> >> +# (generic/567 is a similar test but for punch hole.)
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest auto quick rw zero
> >> +
> >> +# Override the default cleanup function.
> >> +_cleanup()
> >> +{
> >> +	cd /
> >> +	rm -r -f $verifyfile $testfile
> >> +}
> >> +
> >> +# Import common functions.
> >> +. ./common/filter
> >> +
> >> +_require_test
> >> +_require_scratch
> >> +_require_xfs_io_command "fzero"
> >> +
> >> +verifyfile=$TEST_DIR/verifyfile
> >> +testfile=$SCRATCH_MNT/testfile
> >> +
> >> +pagesz=$(getconf PAGE_SIZE)
> >> +
> >> +_scratch_mkfs > /dev/null 2>&1
> >> +_scratch_mount
> >> +
> >> +_dump_files()
> >> +{
> >> +	echo "---- testfile ----"
> >> +	_hexdump $testfile
> >> +	echo "---- verifyfile --"
> >> +	_hexdump $verifyfile
> >> +}
> >> +
> >> +# Build verify file, the data in this file should be consistent with
> >> +# that in the test file.
> >> +$XFS_IO_PROG -f -c "pwrite -S 0x58 0 $((pagesz * 3))" \
> >> +		-c "pwrite -S 0x59 $((pagesz / 2)) $((pagesz * 2))" \
> >> +		$verifyfile | _filter_xfs_io >> /dev/null
> >> +
> >> +# Zero out straddling two pages to check that the mapped write after the
> >> +# range-zeroing are correctly handled.
> >> +$XFS_IO_PROG -t -f \
> >> +	-c "pwrite -S 0x58 0 $((pagesz * 3))" \
> >> +	-c "mmap -rw 0 $((pagesz * 3))" \
> >> +	-c "mwrite -S 0x5a $((pagesz / 2)) $((pagesz * 2))" \
> >> +	-c "fzero $((pagesz / 2)) $((pagesz * 2))" \
> >> +	-c "mwrite -S 0x59 $((pagesz / 2)) $((pagesz * 2))" \
> >> +	-c "close"      \
> > 
> > Hi Zhang,
> > 
> > Thanks for making it work for non-4k pages. Feel free to add:
> > 
> > Reviewed-by: Ojaswin Mujoo <ojaswin@xxxxxxxxxxxxx>
> 
> Hello Ojaswin!
> 
> Thank you very much for your review. I made some minor changes
> based on Darrick's comments and sent out v3 earlier today.
> Perhaps you would like to add your review tag to that one.
> 
> https://lore.kernel.org/fstests/20250108084407.1575909-1-yi.zhang@xxxxxxxxxxxxxxx/

Thanks for pointing it out Zhang, I've reviewed the v3.

Thanks,
ojaswin
> 
> Thanks,
> Yi.
> 




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux