Re: [PATCH 3/3] generic/810: check that RWF_DONTCACHE I/O drops its folios

[Brian Foster <bfoster@redhat.com>]

On Thu, Oct 15, 2026 at 04:54:04PM -0400, Tal Zussman wrote:
> A dontcache write drops its folios from the page cache once writeback
> completes, and a dontcache read drops them once the read completes.
> 
> Test this by writing and reading a 1M file with and without
> RWF_DONTCACHE and checking with mincore how much of it is cached
> afterwards: all of it for plain I/O, none of it for dontcache I/O.
> 
> The overwrite is a separate case because the drop is skipped when
> writeback completes in interrupt context, which is what happens on ext4
> without commit "ext4: complete dropbehind writeback in task context"
> [1].
> 
> Link: https://lore.kernel.org/linux-ext4/20261012-ext4-dontcache-complete-in-task-v1-1-6ac801557041@columbia.edu/ [1]
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  tests/generic/810     | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/810.out | 11 +++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/tests/generic/810 b/tests/generic/810
> new file mode 100755
> index 00000000..9022c26a
> --- /dev/null
> +++ b/tests/generic/810
> @@ -0,0 +1,82 @@
...
> +# Count the bytes of the file that are in the page cache.
> +cached_bytes()
> +{
> +	local pages
> +
> +	pages=$($XFS_IO_PROG -c "mmap -r 0 $size" -c "mincore" $file | \
> +		awk '{ n += $4 } END { print n + 0 }')
> +	echo $((pages * pagesize))
> +}
> +
> +report_cached()
> +{
> +	echo "$(cached_bytes) of $size bytes cached"
> +}
> +
> +# Dontcache writeback drops a folio right after it clears the writeback flag,
> +# so a check immediately after fsync returns can run a moment too early. Give
> +# the drop a little time before reporting.
> +report_cached_after_writeback()
> +{
> +	local i
> +
> +	for ((i = 0; i < 50; i++)); do
> +		[ "$(cached_bytes)" -eq 0 ] && break
> +		sleep 0.1
> +	done
> +	report_cached
> +}

So if I understand this right, the fsync completion isn't technically
sufficient to prevent the don't cache drops from racing with the mincore
check, right?

If so, then we give the system up to 5 seconds or so to report the don't
cache results. That seems reasonable to me, but I find the interface a
little unclear since it seems not universally used for write(back)
cases.

IOW (and I realize this is a wonky example), suppose don't cache breaks
in some way where it starts to apply to cases where it shouldn't.. do we
want to try and test for that?

This is kind of a nit and I'm not opposed to the test as is, so FWIW:

Reviewed-by: Brian Foster <bfoster@redhat.com>

But I do wonder whether this might be a little more robust if we just
gave the writeback case some unconditional settle time before checking
the result. The caveat I suppose is reclaim could still technically
occur at any time and cause false results either way, so *shrug*.

An idea that comes to mind could be to perhaps combine these two
report_cached helpers into one with an optional delay parameter. If not
specified then return the immediate result. If a delay is specified then
still check the initial result, but wait up to $delay seconds for the
result to change. Then maybe the don't cache case could use a little
more aggressive of a delay, but we could still have a short one for the
normal cached case(s). Hm?

Brian

> +
> +echo "plain write"
> +$XFS_IO_PROG -f -c "pwrite -q 0 $size" -c fsync $file
> +report_cached
> +
> +echo "dontcache write to a new file"
> +rm -f $file
> +$XFS_IO_PROG -f -c "pwrite -q -U -V 1 -b $size 0 $size" -c fsync $file
> +report_cached_after_writeback
> +
> +# Start each of the remaining cases from an empty page cache.
> +echo "dontcache overwrite"
> +_scratch_cycle_mount
> +$XFS_IO_PROG -c "pwrite -q -U -V 1 -b $size 0 $size" -c fsync $file
> +report_cached_after_writeback
> +
> +echo "dontcache read"
> +_scratch_cycle_mount
> +$XFS_IO_PROG -c "pread -q -U -V 1 -b $size 0 $size" $file
> +report_cached
> +
> +echo "plain read"
> +_scratch_cycle_mount
> +$XFS_IO_PROG -c "pread -q 0 $size" $file
> +report_cached
> +
> +_scratch_unmount
> +_exit 0
> diff --git a/tests/generic/810.out b/tests/generic/810.out
> new file mode 100644
> index 00000000..cb6c3f2a
> --- /dev/null
> +++ b/tests/generic/810.out
> @@ -0,0 +1,11 @@
> +QA output created by 810
> +plain write
> +1048576 of 1048576 bytes cached
> +dontcache write to a new file
> +0 of 1048576 bytes cached
> +dontcache overwrite
> +0 of 1048576 bytes cached
> +dontcache read
> +0 of 1048576 bytes cached
> +plain read
> +1048576 of 1048576 bytes cached
> 
> -- 
> 2.39.5
> 
>