Re: [PATCH v7 3/5] mm, iomap: keep a partly covered page until an uncached write completes it

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2026-10-20 at 18:03 -0400, Mike Snitzer wrote:
> An uncached (RWF_DONTCACHE) buffered write marks drop-behind every folio
> it creates, so writeback drops each one as soon as this write's data is
> on disk.  A page the write covers only partly is usually shared with the
> neighbouring write, the one that ends or starts in it.  Once the first
> of the two has been written back, the page is gone, and the second has
> to read the block back from disk before it can write its part: one read
> per misaligned write.
> 
> Measured locally on XFS (emulated 4Kn NVMe, 4 KiB pages), 32 threads
> writing interleaved records to one file with pwritev2(), 1 GiB per run,
> before and after this patch:
> 
>   record  flags                     4 KiB device       MiB/s
>   bytes                             reads per write
>                                     before   after     before  after
>   47008   RWF_DONTCACHE             0.97     0.000     831     2281
>   47008   RWF_DONTCACHE|RWF_DSYNC   0.99     0.000     601     1227
>   6000    RWF_DONTCACHE             0.97     0.000     121      488
>   6000    RWF_DONTCACHE|RWF_DSYNC   0.99     0.000      90      206
> 
> Before this patch every write waited for its read, and because XFS holds
> its IOLOCK exclusively across a buffered write, the other 31 writers
> waited too: both record sizes ran at about 20 thousand writes a second,
> one device read per write; after it nothing is read back.  With
> RWF_DSYNC each write still waits for its own sync, so the gain is
> smaller: the uncached write now runs as fast as the same write without
> RWF_DONTCACHE, 1237 and 209 MiB/s against 1227 and 206 in the table.
> Records of 49152 bytes, which cover only whole pages, read nothing
> before or after.
> 
> NFSD DIRECT issues the misaligned ends of a WRITE as buffered uncached
> segments and gains the same way.  Without any handling of the shared
> page, 32 interleaved writers of 47008-byte records read 45144 blocks
> for 45664 WRITEs; with this patch, and no NFSD code for the shared page,
> 0.0001 to 0.0008 blocks per WRITE whether each record is sent as one
> FILE_SYNC WRITE, as two one after the other, as two together, or as two
> UNSTABLE.  The page cache then marks the completed page while it is
> still dirty, so each segment of a stable WRITE wakes the DONTCACHE
> writeback kick (23 to 65 thousand rounds a run); the next two patches
> let NFSD mark it after its sync instead.
> 
> Let the write path say which pages a write covers only partly
> (FGP_DONTCACHE_EDGE, set by iomap when the write starts inside a page or
> ends before its end) and get such a page as a single page.  A folio
> created for it is not marked, so writing back this write's part does not
> drop it.  A folio found uptodate is the neighbour's, already written:
> this write completes it, so mark it, under the folio lock and before its
> own data goes in, and the writeback that cleans both writes' data drops
> it.  The folio lock orders the two writes, so the second always finds
> the first's data in place.
> 
> A page whose neighbour never writes, such as the last page of a file,
> stays cached until reclaim.  A partly covered page another task had
> cached and that is uptodate is now dropped after the write's writeback,
> where before an uncached write never dropped a page it did not create.
> 
> Only iomap file systems say which pages a write covers partly so far;
> write_begin_get_folio() users such as ext4 and the NFS client do not.
> With blocks smaller than a page, a write that crosses an extent boundary
> inside a page asks for it as two partly covered halves, so the page,
> though one write covers it whole, stays cached until reclaim.
> 
> Assisted-by: LLM
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  fs/iomap/buffered-io.c  | 12 +++++++++++-
>  include/linux/pagemap.h |  5 +++++
>  mm/filemap.c            | 19 ++++++++++++++++++-
>  3 files changed, 34 insertions(+), 2 deletions(-)
> 

The nfsd case typically uses buffered or DONTCACHE for the unaligned
sub-page-sized "ends" when direct is in force, so mostly these ends get
submitted as small (< PAGE_SIZE or folio_size()) buffered I/Os.

Why do we need this special casing instead of just making those
unaligned ends use regular buffered I/O? Isn't it basically the same
thing?
 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 0a5ebfda90f1..780a0186045d 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -802,8 +802,18 @@ struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos, size_t len)
>  
>  	if (iter->flags & IOMAP_NOWAIT)
>  		fgp |= FGP_NOWAIT;
> -	if (iter->flags & IOMAP_DONTCACHE)
> +	if (iter->flags & IOMAP_DONTCACHE) {
>  		fgp |= FGP_DONTCACHE;
> +		/*
> +		 * A page this write covers only partly is shared with a
> +		 * neighbouring write: get it as a single page, which is
> +		 * kept until the write that completes it.
> +		 */
> +		if (offset_in_page(pos) || len < PAGE_SIZE) {
> +			fgp |= FGP_DONTCACHE_EDGE;
> +			len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
> +		}
> +	}
>  	fgp |= fgf_set_order(len);
>  
>  	return __filemap_get_folio(iter->inode->i_mapping, pos >> PAGE_SHIFT,
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 3012bd95812a..0c1d6bd09aa0 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -701,6 +701,10 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
>   * * %FGP_NOWAIT - Don't block on the folio lock.
>   * * %FGP_STABLE - Wait for the folio to be stable (finished writeback)
>   * * %FGP_DONTCACHE - Uncached buffered IO
> + * * %FGP_DONTCACHE_EDGE - With %FGP_DONTCACHE: the write covers this page
> + *   only partly, and a neighbouring write may complete it.  A folio created
> + *   for it is not marked drop-behind; a folio found uptodate is, as the
> + *   write that completes it.
>   * * %FGP_WRITEBEGIN - The flags to use in a filesystem write_begin()
>   *   implementation.
>   */
> @@ -715,6 +719,7 @@ typedef unsigned int __bitwise fgf_t;
>  #define FGP_FOR_MMAP		((__force fgf_t)0x00000040)
>  #define FGP_STABLE		((__force fgf_t)0x00000080)
>  #define FGP_DONTCACHE		((__force fgf_t)0x00000100)
> +#define FGP_DONTCACHE_EDGE	((__force fgf_t)0x00000200)
>  #define FGF_GET_ORDER(fgf)	(((__force unsigned)fgf) >> 26)	/* top 6 bits */
>  
>  #define FGP_WRITEBEGIN		(FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE)
> diff --git a/mm/filemap.c b/mm/filemap.c
> index ed7e4dbea653..0397ec771547 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2016,6 +2016,16 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
>  
>  	if (fgp_flags & FGP_STABLE)
>  		folio_wait_stable(folio);
> +
> +	/*
> +	 * An uncached write that covers this folio only partly, finding it
> +	 * uptodate, is the write that completes it: the neighbouring write
> +	 * that left it here has written its part.  Mark it now, under the
> +	 * folio lock, so the writeback that cleans both writes' data drops it.
> +	 */
> +	if ((fgp_flags & FGP_DONTCACHE) && (fgp_flags & FGP_DONTCACHE_EDGE) &&
> +	    (fgp_flags & FGP_LOCK) && folio_test_uptodate(folio))
> +		folio_mark_dropbehind(folio);
>  no_page:
>  	if (!folio && (fgp_flags & FGP_CREAT)) {
>  		unsigned int min_order = mapping_min_folio_order(mapping);
> @@ -2053,7 +2063,14 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
>  			/* Init accessed so avoid atomic mark_page_accessed later */
>  			if (fgp_flags & FGP_ACCESSED)
>  				__folio_set_referenced(folio);
> -			if (fgp_flags & FGP_DONTCACHE)
> +			/*
> +			 * A folio the write covers only partly is left
> +			 * unmarked, so that writing back this write's part
> +			 * does not drop it before a neighbouring write has
> +			 * written the rest.
> +			 */
> +			if ((fgp_flags & FGP_DONTCACHE) &&
> +			    !(fgp_flags & FGP_DONTCACHE_EDGE))
>  				__folio_set_dropbehind(folio);
>  
>  			err = filemap_add_folio(mapping, folio, index, gfp);

-- 
Jeff Layton <jlayton@kernel.org>