Re: [PATCH v2] fstests: generic: add a basic cachestat test case

Qu Wenruo <[email protected]> Thu, 16 Jul 2026 07:53:51 +0930
Newsgroups org.kernel.vger.fstests,org.kernel.vger.linux-btrfs
Message-ID <[email protected]>

在 2026/7/16 05:10, Brian Foster 写道:
> On Tue, Jul 07, 2026 at 09:04:54PM +0930, Qu Wenruo wrote:
>> The test case is inspired by LTP, where there is a regression on 64K
>> page size systems with btrfs, that after a fsync, cachestat() still
>> report dirty pages.
>>
>> The test case itself is pretty simple, fill the file with a buffered write that is
>> 1/2/4/8/16 page sized, call cachestat() to make sure the cached/dirtied
>> number match the page number.
>>
>> Then do a fsync(), and make sure the dirty page number reduced to 0
>> meanwhile cached is still the same.
>>
>> Link: https://bugzilla.suse.com/show_bug.cgi?id=1270397
>> Reviewed-by: Anand Jain <[email protected]>
>> Signed-off-by: Qu Wenruo <[email protected]>
>> ---
>> Changelog:
>> v2:
>> - Reject "sync" and "dax" mount options
>>    Those mount options write back data synchronously, will screw up the
>>    buffered write dirtied page reporting.
>>
>> - Use AWK_PROG everywhere.
>>
>> - Remove the unnecessary redirection for fsync
>> ---
>>   tests/generic/798     | 61 +++++++++++++++++++++++++++++++++++++++++++
>>   tests/generic/798.out |  2 ++
>>   2 files changed, 63 insertions(+)
>>   create mode 100755 tests/generic/798
>>   create mode 100644 tests/generic/798.out
>>
>> diff --git a/tests/generic/798 b/tests/generic/798
>> new file mode 100755
>> index 00000000..8888f6a3
>> --- /dev/null
>> +++ b/tests/generic/798
>> @@ -0,0 +1,61 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
>> +#
>> +# FS QA Test 798
>> +#
>> +# Basic tests for cachestat()
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick
>> +
>> +_require_xfs_io_command "cachestat"
>> +_require_scratch
>> +# Any mount option that writes data back synchronously should be rejected.
>> +# Or it will screw up the dirtied page reporting for buffered writes.
>> +_exclude_scratch_mount_option "dax"
>> +_exclude_scratch_mount_option "sync"
>> +
>> +pagesize=$(_get_page_size)
>> +
>> +for num_page in 1 2 4 8 16; do
>> +	size=$(($pagesize * $num_page))
>> +
>> +	echo "=== Test with $num_page pages ===" >> $seqres.full
>> +	_scratch_mkfs > /dev/null
>> +	_scratch_mount
>> +
> 
> Do we need a mkfs and remount for each size we're testing? It seems like
> we should be able to mkfs and mount once and then just remove the file
> per iteration.
> 
>> +	# Basic cached number reporting
>> +	$XFS_IO_PROG -f -c "pwrite -b $pagesize 0 $size" \
>> +		$SCRATCH_MNT/foobar >> $seqres.full
>> +	$XFS_IO_PROG -c "cachestat 0 $size" $SCRATCH_MNT/foobar > $tmp.output
>> +	cat $tmp.output >> $seqres.full
>> +	cached=$(cat $tmp.output | cut -f1 -d, | $AWK_PROG '{print $2}')
>> +	dirtied=$(cat $tmp.output | cut -f2 -d, | $AWK_PROG '{print $2}')
>> +
>> +	if [ "$cached" -ne "$num_page" ]; then
>> +		_fail "cached not matching the page number"
>> +	fi
>> +
>> +	if [ "$cached" -ne "$dirtied" ]; then
>> +		_fail "dirtied not matching the page number"
>> +	fi
> 
> We should expect consistent values for cached and dirtied throughout the
> test, right? If so, it might be more useful (and maybe more simple) to
> just print the cachestat output on each iteration and let the comparison
> to the golden .out file determine pass or failure. Hm?

Right, will update the test case the address both comments.

Thanks,
Qu

> 
> Brian
> 
>> +	$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/foobar
>> +
>> +	# Test dirty page number reporting after a fsync.
>> +	$XFS_IO_PROG -c "cachestat 0 $size" $SCRATCH_MNT/foobar > $tmp.output
>> +	cat $tmp.output >> $seqres.full
>> +	_scratch_unmount
>> +	cached=$(cat $tmp.output | cut -f1 -d, | $AWK_PROG '{print $2}')
>> +	dirtied=$(cat $tmp.output | cut -f2 -d, | $AWK_PROG '{print $2}')
>> +
>> +	if [ "$cached" -ne "$num_page" ]; then
>> +		_fail "cached not matching the page number"
>> +	fi
>> +	if [ "$dirtied" -ne 0 ]; then
>> +		_fail "dirtied pages not zero"
>> +	fi
>> +done
>> +
>> +echo "Silence is golden"
>> +_exit 0
>> diff --git a/tests/generic/798.out b/tests/generic/798.out
>> new file mode 100644
>> index 00000000..216d6e93
>> --- /dev/null
>> +++ b/tests/generic/798.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 798
>> +Silence is golden
>> -- 
>> 2.51.2
>>
>>
>