[PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages

[PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages

[Mike Snitzer]

Hi,

Please consider this v8 for the v7.5 development cycle, so that it can
go upstream in the v7.5 merge window.

An uncached (RWF_DONTCACHE) buffered write drops each page it writes
once that page is written back.  A page the write covers only partly is
shared with the neighbouring write, and once the first of the two has
been written back the second has to read it back from disk before it
can write its part.  32 threads writing interleaved records to one file
on XFS (emulated 4Kn NVMe, 4 KiB pages), before and after 5/7:

  record  pwritev2() flags          4 KiB device       MiB/s
  bytes                             reads per write
                                    before   after     before  after
  47008   RWF_DONTCACHE             0.97     0.0005    831     2281
  47008   RWF_DONTCACHE|RWF_DSYNC   0.99     0.0005    601     1227
  6000    RWF_DONTCACHE             0.97     0.0001    121      488
  6000    RWF_DONTCACHE|RWF_DSYNC   0.99     0.0002     90      206

What is left after is metadata: 49152-byte records, which cover only
whole pages, read 0.0005 per write either way.  The test program and
script that produced this table, and how to run them, are in:
https://lore.kernel.org/linux-nfs/atqJBS8MlYfeUU-t@kernel.org/

5/7 has the page cache keep such a page for the write that completes
it.  6/7 lets a writer that syncs its own write, as NFSD does for a
stable WRITE, complete those pages itself once the sync is done.  7/7
is the NFSD change that uses it.  3/7 reports the DONTCACHE dirty count
in the bdi debugfs stats, and 4/7 adds folio_mark_dropbehind(), which
5/7 and 6/7 use.

1/7 and 2/7 are fixes the review of v7 turned up in code that is
already queued or upstream, and each stands on its own:

- 1/7 fixes 80728157d1bd ("NFSD: cut a misaligned direct-mode WRITE at
  page boundaries") in cel/nfsd-testing: where the mapping's smallest
  folio is a block larger than a page, NFSD cut a WRITE's misaligned
  ends inside a block, so the direct middle and a buffered end shared
  it.  Chuck, maybe best to fold into 80728157d1bd ?
- 2/7 fixes 88d6f128d06d ("mm: track DONTCACHE dirty pages per
  bdi_writeback"), in v7.3: a cached lookup that clears the dropbehind
  flag could take a folio off WB_DONTCACHE_DIRTY that was never counted
  there, or leave one counted with nothing to uncount it.  It applies
  to v7.3 as it is, if the mm side would rather take it alone.

The prerequisite NFSD changes are staged in cel/nfsd-testing; this
series applies on top of it at 6d13b7f74eec ("NFSD: Map every NFS_ACLv2
reply status into aclstat2"), and each patch builds with W=1 with no
warnings.

2/7 to 6/7 are mm, with small iomap changes in 5/7 and 6/7, and are
Cc'd to linux-mm and linux-fsdevel.  If the mm and iomap maintainers
are agreeable, their acks would let Chuck take the series through the
nfsd tree.  But whatever approach the maintainers prefer for this kind
of cross-subsystem merge is obviously fine.

Changes since v7, from Jeff's review and the sashiko findings he
relayed:
- 1/7 and 2/7 are new (above).
- 3/7 (v7 1/5): a comment dropped; Jeff's Reviewed-by.
- 4/7 (v7 2/5): folio_mark_dropbehind() counted a folio as DONTCACHE
  dirty when it found PG_dirty set, but PG_dirty is set before
  folio_account_dirtied() counts the folio, without the folio lock, so
  a folio marked in between was counted twice.  It now sets the flag
  and tests the dirty tag, which is set in the same i_pages section that
  counts the folio, under the i_pages lock.
- 5/7 (v7 3/5): the edge is the smallest folio the mapping allows, a
  page or a block where blocks are larger, both in iomap's test and in
  what is marked: a larger folio found there, left by readahead or
  another buffered write, is no longer marked whole.
- 6/7 (v7 4/5): an edge already clean is dropped before the folio lock
  is released, not after with a trylock that could miss it; and only
  an edge-sized folio, as in 5/7.
- 7/7 (v7 5/5): unchanged.

Tested on XFS on an emulated 4Kn NVMe, 4 KiB pages, 32 threads of
interleaved 47008-byte records (12288 bytes where noted), 4 KiB device
reads per write, v7 against v8:

                                             v7        v8
  RWF_DONTCACHE over large folios
    readahead left                          0.94      0.0026
  RWF_DONTCACHE, 16 KiB blocks              1.62      0.0014
    12288-byte records                      2.89      0.0004
  NFS WRITEs through NFSD DIRECT,
    16 KiB blocks                           3.46      0.0016

(v7 in 128 MiB runs, v8 in 1 GiB runs; on 16 KiB blocks one block read
back per write is 4.0 here.)  In every run of v8, BdiDontcacheDirty
(3/7) was back at 0 after the final sync, including runs mixing
uncached writes, cached writes and mmap stores on one file, and every
record verified.

v7: https://lore.kernel.org/linux-nfs/20261020220346.28020-1-snitzer@kernel.org/
v6: https://lore.kernel.org/linux-nfs/atbfEbgMU6UGkn2X@kernel.org/
v5: https://lore.kernel.org/linux-nfs/20261019150725.1507-1-snitzer@kernel.org/
v4: https://lore.kernel.org/linux-nfs/20261015233309.42999-1-snitzer@kernel.org/
v3: https://lore.kernel.org/linux-nfs/20261001045502.48381-1-snitzer@kernel.org/
v2: https://lore.kernel.org/linux-nfs/20260929231329.22018-1-snitzer@kernel.org/
v1: https://lore.kernel.org/linux-nfs/20260929173423.16149-1-snitzer@kernel.org/

Thanks,
Mike

Mike Snitzer (7):
  NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio
  mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there
  mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats
  mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it
  mm, iomap: keep a partly covered page until an uncached write completes it
  mm, iomap: let an uncached writer complete its shared pages itself
  NFSD: drop a shared boundary page only once both WRITEs have written it

 .../filesystems/nfs/nfsd-io-modes.rst         |  17 ++
 fs/iomap/buffered-io.c                        |  20 +-
 fs/iomap/trace.h                              |   3 +-
 fs/nfsd/vfs.c                                 | 113 ++++++++--
 include/linux/fs.h                            |   8 +-
 include/linux/iomap.h                         |   1 +
 include/linux/pagemap.h                       |  12 ++
 mm/backing-dev.c                              |   6 +
 mm/filemap.c                                  | 193 ++++++++++++++++--
 9 files changed, 338 insertions(+), 35 deletions(-)


base-commit: 6d13b7f74eec16e08d5e7f772ee119432a14ef2c
-- 
2.52.0

[PATCH v8 1/7] NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio

[Mike Snitzer]

nfsd_write_dio_iters_init() cuts a misaligned end of a direct-mode
WRITE at a multiple of the larger of offset_align and PAGE_SIZE, so
that the direct middle, which invalidates the page cache over its
range, never shares a page with a buffered end.  But where the
mapping's minimum folio order makes the smallest folio a block larger
than a page, the page cache holds that block as one folio: an end cut
on a page boundary inside a block leaves the middle and the buffered
end in the same folio.  The middle's invalidation then drops the folio
the end wrote, and whichever of the two neighbouring WRITEs comes second
reads the block back.

Cut a misaligned end at a multiple of the smallest folio the mapping
allows as well.  Where that is a page, nothing changes.

Fixes: 80728157d1bd ("NFSD: cut a misaligned direct-mode WRITE at page boundaries")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/vfs.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 9219f8eb386d..ade00dabef18 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1452,13 +1452,15 @@ nfsd_write_dio_iters_init(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	/*
 	 * The direct middle invalidates the page cache over its range, so it
-	 * must not share a page with a buffered end.  An aligned end needs no
-	 * buffered segment; a misaligned one is cut on a page boundary even
-	 * where the offset alignment is finer than a page.  A WRITE may end
-	 * at OFFSET_MAX, so the prefix is sized as a distance rather than
-	 * by rounding @offset up, which could wrap.
+	 * must not share a folio with a buffered end.  An aligned end needs no
+	 * buffered segment; a misaligned one is cut on a boundary of the
+	 * smallest folio the mapping allows, a page or, where the block is
+	 * larger, a block, even where the offset alignment is finer.  A WRITE
+	 * may end at OFFSET_MAX, so the prefix is sized as a distance rather
+	 * than by rounding @offset up, which could wrap.
 	 */
-	cut = max_t(u32, offset_align, PAGE_SIZE);
+	cut = max3(offset_align, PAGE_SIZE,
+		   mapping_min_folio_nrbytes(nf->nf_file->f_mapping));
 	prefix_end = offset;
 	if (!IS_ALIGNED(offset, offset_align))
 		prefix_end += min_t(loff_t, cut - (offset & (cut - 1)), total);
-- 
2.52.0

[PATCH v8 2/7] mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there

[Mike Snitzer]

A cached lookup in __filemap_get_folio_mpol() that finds a folio marked
dropbehind clears the flag, and subtracts the folio from
WB_DONTCACHE_DIRTY if PG_dirty is set.  But filemap_dirty_folio() sets
PG_dirty before folio_account_dirtied() counts the folio, under the
i_pages lock and without the folio lock.  A lookup between the two
subtracts a folio that was never added, and folio_account_dirtied(),
finding the flag clear, does not add it either, so the counter the
DONTCACHE writeback kick reads falls short and can go negative.  A
lookup without FGP_LOCK also races with the paths that uncount a dirty
folio, which hold the folio lock: folio_clear_dirty_for_io() clears
PG_dirty and then tests the flag, so a lookup between the two leaves
the folio counted with no one to uncount it.

Every path that uncounts a dirty folio holds the folio lock, and
folio_account_dirtied() sets the folio's dirty tag in the same i_pages
section that counts it.  So clear the flag with the folio locked,
taking the lock with a trylock for a lookup without FGP_LOCK, and test
the dirty tag rather than PG_dirty under the i_pages lock: the folio
is uncounted only if it was counted.  Under that lock the wb is
inode_to_wb(), as in folio_account_dirtied() and inode_do_switch_wbs(),
which moves the counter by the same tag.  A lookup that finds the
folio locked by another task leaves the flag set, as it would have
been without the lookup.

Fixes: 88d6f128d06d ("mm: track DONTCACHE dirty pages per bdi_writeback")
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 mm/filemap.c | 54 +++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 00fd89cf6f55..1cc28224d547 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1927,6 +1927,45 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index)
 	return folio;
 }
 
+/*
+ * Clear the uncached flag of a folio a cached lookup found, and take the
+ * folio off WB_DONTCACHE_DIRTY if it is counted there.
+ *
+ * folio_account_dirtied() counts a dropbehind folio under the i_pages lock,
+ * in the section that sets its dirty tag, after PG_dirty has been set
+ * without the folio lock, and every path that uncounts a dirty folio holds
+ * the folio lock.  So with the folio locked and the i_pages lock held, the
+ * dirty tag, not PG_dirty, says whether the folio is counted: a dirtier past
+ * folio_test_set_dirty() but not yet counting finds the flag clear and does
+ * not count it.  A folio another task has locked keeps the flag, as it
+ * would have had this lookup not run.
+ */
+static void filemap_clear_dropbehind(struct address_space *mapping,
+				     struct folio *folio, bool locked)
+{
+	unsigned long flags;
+
+	if (!locked) {
+		if (!folio_trylock(folio))
+			return;
+		if (unlikely(folio->mapping != mapping))
+			goto out;
+	}
+	if (!mapping_can_writeback(mapping)) {
+		folio_clear_dropbehind(folio);
+		goto out;
+	}
+	xa_lock_irqsave(&mapping->i_pages, flags);
+	if (folio_test_clear_dropbehind(folio) &&
+	    xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY))
+		wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY,
+			    -folio_nr_pages(folio));
+	xa_unlock_irqrestore(&mapping->i_pages, flags);
+out:
+	if (!locked)
+		folio_unlock(folio);
+}
+
 /**
  * __filemap_get_folio_mpol - Find and get a reference to a folio.
  * @mapping: The address_space to search.
@@ -2058,19 +2097,8 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
 	if (!folio)
 		return ERR_PTR(-ENOENT);
 	/* not an uncached lookup, clear uncached if set */
-	if (!(fgp_flags & FGP_DONTCACHE) && folio_test_clear_dropbehind(folio)) {
-		if (folio_test_dirty(folio) &&
-		    mapping_can_writeback(mapping)) {
-			struct inode *inode = mapping->host;
-			struct bdi_writeback *wb;
-			struct wb_lock_cookie cookie = {};
-			long nr = folio_nr_pages(folio);
-
-			wb = unlocked_inode_to_wb_begin(inode, &cookie);
-			wb_stat_mod(wb, WB_DONTCACHE_DIRTY, -nr);
-			unlocked_inode_to_wb_end(inode, &cookie);
-		}
-	}
+	if (!(fgp_flags & FGP_DONTCACHE) && folio_test_dropbehind(folio))
+		filemap_clear_dropbehind(mapping, folio, fgp_flags & FGP_LOCK);
 	return folio;
 }
 EXPORT_SYMBOL(__filemap_get_folio_mpol);
-- 
2.52.0

[PATCH v8 3/7] mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats

[Mike Snitzer]

The per-bdi_writeback WB_DONTCACHE_DIRTY counter drives how much the
flusher writes back in response to an IOCB_DONTCACHE kick, but it is
not reported anywhere, so the DONTCACHE dirty backlog can only be
inferred from MemFree/Cached movement.

Report it as BdiDontcacheDirty in /sys/kernel/debug/bdi/<dev>/stats
and as WbDontcacheDirty in the per-wb stats, next to the
Reclaimable line.  Use wb_stat_sum() rather than wb_stat() to fold in
the per-cpu batches: the flusher targets the summed value and small
backlogs would otherwise read as zero.

Fixes: 88d6f128d06d ("mm: track DONTCACHE dirty pages per bdi_writeback")
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 mm/backing-dev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 18e999053bae..111b45262979 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -46,6 +46,7 @@ struct wb_stats {
 	unsigned long nr_dirty_time;
 	unsigned long nr_writeback;
 	unsigned long nr_reclaimable;
+	unsigned long nr_dontcache_dirty;
 	unsigned long nr_dirtied;
 	unsigned long nr_written;
 	unsigned long dirty_thresh;
@@ -78,6 +79,7 @@ static void collect_wb_stats(struct wb_stats *stats,
 
 	stats->nr_writeback += wb_stat(wb, WB_WRITEBACK);
 	stats->nr_reclaimable += wb_stat(wb, WB_RECLAIMABLE);
+	stats->nr_dontcache_dirty += wb_stat_sum(wb, WB_DONTCACHE_DIRTY);
 	stats->nr_dirtied += wb_stat(wb, WB_DIRTIED);
 	stats->nr_written += wb_stat(wb, WB_WRITTEN);
 	stats->wb_thresh += wb_calc_thresh(wb, stats->dirty_thresh);
@@ -125,6 +127,7 @@ static int bdi_debug_stats_show(struct seq_file *m, void *v)
 	seq_printf(m,
 		   "BdiWriteback:       %10lu kB\n"
 		   "BdiReclaimable:     %10lu kB\n"
+		   "BdiDontcacheDirty:  %10lu kB\n"
 		   "BdiDirtyThresh:     %10lu kB\n"
 		   "DirtyThresh:        %10lu kB\n"
 		   "BackgroundThresh:   %10lu kB\n"
@@ -139,6 +142,7 @@ static int bdi_debug_stats_show(struct seq_file *m, void *v)
 		   "state:              %10lx\n",
 		   K(stats.nr_writeback),
 		   K(stats.nr_reclaimable),
+		   K(stats.nr_dontcache_dirty),
 		   K(stats.wb_thresh),
 		   K(dirty_thresh),
 		   K(background_thresh),
@@ -163,6 +167,7 @@ static void wb_stats_show(struct seq_file *m, struct bdi_writeback *wb,
 		   "WbCgIno:           %10lu\n"
 		   "WbWriteback:       %10lu kB\n"
 		   "WbReclaimable:     %10lu kB\n"
+		   "WbDontcacheDirty:  %10lu kB\n"
 		   "WbDirtyThresh:     %10lu kB\n"
 		   "WbDirtied:         %10lu kB\n"
 		   "WbWritten:         %10lu kB\n"
@@ -179,6 +184,7 @@ static void wb_stats_show(struct seq_file *m, struct bdi_writeback *wb,
 #endif
 		   K(stats->nr_writeback),
 		   K(stats->nr_reclaimable),
+		   K(stats->nr_dontcache_dirty),
 		   K(stats->wb_thresh),
 		   K(stats->nr_dirtied),
 		   K(stats->nr_written),
-- 
2.52.0

[PATCH v8 4/7] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it

[Mike Snitzer]

A folio allocated by an RWF_DONTCACHE write is marked dropbehind before
it is dirtied, so folio_account_dirtied() adds it to WB_DONTCACHE_DIRTY
and folio_clear_dirty_for_io() or folio_account_cleaned() subtracts it
when it is cleaned.  Nothing marks a folio that already exists.  A
caller that sets PG_dropbehind on a folio that is already dirty
unbalances the counter: the clean subtracts what was never added, and
once the counter goes negative, wb_stat_sum() reads it as zero and the
DONTCACHE writeback kick stops writing anything back for every writer
on that bdi_writeback.

Add folio_mark_dropbehind(), the inverse of the clear in
__filemap_get_folio_mpol(): with the folio locked, mark it and, if it
has been counted as dirty, add it to WB_DONTCACHE_DIRTY.  PG_dirty is
not the test for that: filemap_dirty_folio() sets it before
folio_account_dirtied() counts the folio, under the i_pages lock and
without the folio lock, so a folio marked between the two would be
counted twice.  folio_account_dirtied() sets the folio's dirty tag in
the same i_pages section that counts it, and every path that uncounts a
dirty folio holds the folio lock, so set the flag and test the dirty tag
under the i_pages lock: a dirtier not yet counting the folio finds the
flag set and counts it itself.  Under that lock the wb is inode_to_wb(),
as in folio_account_dirtied().  A concurrent lookup without
FGP_DONTCACHE that clears the flag again subtracts what this added.

The name follows folio_set_dirty() and folio_mark_dirty(): the first
only sets PG_dirty, the second also accounts the dirty folio.
folio_set_dropbehind() likewise only sets PG_dropbehind, and
folio_mark_dropbehind() also accounts it in WB_DONTCACHE_DIRTY.

A later commit uses it in NFSD to drop a page shared by two misaligned
WRITEs once both have written it.

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 include/linux/pagemap.h |  1 +
 mm/filemap.c            | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 0adfa6605653..3012bd95812a 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -1310,6 +1310,7 @@ void end_page_writeback(struct page *page);
 void folio_end_writeback(struct folio *folio);
 void folio_end_writeback_no_dropbehind(struct folio *folio);
 void folio_end_dropbehind(struct folio *folio);
+void folio_mark_dropbehind(struct folio *folio);
 void folio_wait_stable(struct folio *folio);
 void __folio_mark_dirty(struct folio *folio, struct address_space *, int warn);
 void folio_account_cleaned(struct folio *folio, struct bdi_writeback *wb);
diff --git a/mm/filemap.c b/mm/filemap.c
index 1cc28224d547..0c76ee5750b3 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1644,6 +1644,45 @@ void folio_end_dropbehind(struct folio *folio)
 }
 EXPORT_SYMBOL_GPL(folio_end_dropbehind);
 
+/**
+ * folio_mark_dropbehind - mark an existing folio dropbehind
+ * @folio: the folio, locked by the caller
+ *
+ * Mark @folio so that the writeback that cleans it drops it.  A folio
+ * already counted as dirty is added to WB_DONTCACHE_DIRTY, as
+ * folio_account_dirtied() would have added it had it been marked when
+ * dirtied, so that the clean that subtracts it leaves the counter balanced.
+ *
+ * folio_account_dirtied() counts a folio under the i_pages lock, in the
+ * section that sets its dirty tag, after PG_dirty has been set without the
+ * folio lock, and every path that uncounts a dirty folio holds the folio
+ * lock.  So with the folio locked and the i_pages lock held, the dirty tag,
+ * not PG_dirty, says whether the folio has been counted: a dirtier past
+ * folio_test_set_dirty() but not yet counting finds the flag set and adds
+ * the folio itself.
+ */
+void folio_mark_dropbehind(struct folio *folio)
+{
+	struct address_space *mapping = folio->mapping;
+	unsigned long flags;
+
+	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
+
+	if (folio_test_dropbehind(folio))
+		return;
+	if (!mapping || !mapping_can_writeback(mapping)) {
+		folio_set_dropbehind(folio);
+		return;
+	}
+	xa_lock_irqsave(&mapping->i_pages, flags);
+	folio_set_dropbehind(folio);
+	if (xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY))
+		wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY,
+			    folio_nr_pages(folio));
+	xa_unlock_irqrestore(&mapping->i_pages, flags);
+}
+EXPORT_SYMBOL_GPL(folio_mark_dropbehind);
+
 /**
  * folio_end_writeback_no_dropbehind - End writeback against a folio.
  * @folio: The folio.
-- 
2.52.0

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

[Mike Snitzer]

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.0005    831     2281
  47008   RWF_DONTCACHE|RWF_DSYNC   0.99     0.0005    601     1227
  6000    RWF_DONTCACHE             0.97     0.0001    121      488
  6000    RWF_DONTCACHE|RWF_DSYNC   0.99     0.0002     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 only metadata is read.  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 only that
metadata, 0.0005 per write, before and 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) and get such a page as the smallest folio the
mapping allows: a single page, or a single block where the mapping's
minimum folio order makes the block larger than a page, which is then
the unit two writes share.  iomap sets it when the write starts inside
that unit or ends before its end, testing against
mapping_min_folio_nrbytes().  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 folio found there that is larger than that unit, left by
readahead or another buffered write, holds more than the two writes
share and cannot be dropped in part: it is not marked, and stays cached
as it would without this change.

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.

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/iomap/buffered-io.c  | 16 +++++++++++++++-
 include/linux/pagemap.h |  5 +++++
 mm/filemap.c            | 34 +++++++++++++++++++++++++++++++++-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 0a5ebfda90f1..86f884c9f8f2 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -802,8 +802,22 @@ 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) {
+		size_t unit = mapping_min_folio_nrbytes(iter->inode->i_mapping);
+		size_t off = pos & (unit - 1);
+
 		fgp |= FGP_DONTCACHE;
+		/*
+		 * The smallest folio the mapping allows, a page or a block
+		 * where the block is larger, that this write covers only
+		 * partly is shared with a neighbouring write: get it as just
+		 * that, which is kept until the write that completes it.
+		 */
+		if (off || len < unit) {
+			fgp |= FGP_DONTCACHE_EDGE;
+			len = min_t(size_t, len, unit - off);
+		}
+	}
 	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 0c76ee5750b3..1494e42e609f 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1683,6 +1683,18 @@ void folio_mark_dropbehind(struct folio *folio)
 }
 EXPORT_SYMBOL_GPL(folio_mark_dropbehind);
 
+/*
+ * Is @folio the unit two writes share at an edge: the smallest folio the
+ * mapping allows, a single page or, where the block is larger, a single
+ * block?  A larger folio, left by readahead or another buffered write, holds
+ * more than that unit and is not marked for an edge.
+ */
+static bool filemap_dontcache_edge_unit(struct address_space *mapping,
+					struct folio *folio)
+{
+	return folio_nr_pages(folio) == mapping_min_folio_nrpages(mapping);
+}
+
 /**
  * folio_end_writeback_no_dropbehind - End writeback against a folio.
  * @folio: The folio.
@@ -2063,6 +2075,19 @@ 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.
+	 * Not a folio larger than the edge the two writes share, which holds
+	 * more than they wrote and cannot be dropped in part.
+	 */
+	if ((fgp_flags & FGP_DONTCACHE) && (fgp_flags & FGP_DONTCACHE_EDGE) &&
+	    (fgp_flags & FGP_LOCK) && folio_test_uptodate(folio) &&
+	    filemap_dontcache_edge_unit(mapping, folio))
+		folio_mark_dropbehind(folio);
 no_page:
 	if (!folio && (fgp_flags & FGP_CREAT)) {
 		unsigned int min_order = mapping_min_folio_order(mapping);
@@ -2100,7 +2125,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);
-- 
2.52.0

[PATCH v8 6/7] mm, iomap: let an uncached writer complete its shared pages itself

[Mike Snitzer]

The write that completes a page an uncached write covers only partly
marks it drop-behind as it writes, while the page is dirty, so the page
counts as DONTCACHE dirty and the DONTCACHE writeback kick writes it
back.  A writer that syncs its write itself, once, after several
segments -- NFSD answering a stable WRITE, the NFS client's LOCALIO path
-- issues each segment without IOCB_DSYNC, so every segment's kick
writes back pages its own sync is about to write.  Measured with NFSD
DIRECT and 32 writers of 47008-byte records: with the previous patch
alone, 23,000 to 65,000 kick rounds a run, where NFSD marking the pages
itself after its sync had taken 51 to 274; with this patch and NFSD
using it, a FILE_SYNC WRITE wakes the kick not at all.

Let such a writer complete those pages itself.  IOCB_DONTCACHE_DEFER
(IOMAP_DONTCACHE_DEFER, FGP_DONTCACHE_DEFER) leaves a partly covered page
unmarked even when the write finds it uptodate.  Right before writing such
a page the writer asks filemap_dontcache_edge_completes() whether it is
uptodate already, which means the neighbouring write has written its part
and this write completes it; once the write and its sync are done,
filemap_dontcache_edge_complete() marks it and, clean, drops it at once,
before it lets go of the folio lock: dropping it after the unlock, with
folio_end_dropbehind()'s trylock, would leave a page another task locks
in between for reclaim, no writeback being left to drop it.  As in the
default path, both helpers take only a folio the smallest the mapping
allows; a larger one is left cached.

Two writes that arrive together may both find the page absent before
either has written; that page is then left cached until reclaim, which
the folio-lock ordering of the default path avoids.

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/iomap/buffered-io.c  |  4 +++
 fs/iomap/trace.h        |  3 +-
 include/linux/fs.h      |  8 ++++-
 include/linux/iomap.h   |  1 +
 include/linux/pagemap.h |  6 ++++
 mm/filemap.c            | 70 +++++++++++++++++++++++++++++++++++++++--
 6 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 86f884c9f8f2..78d560a7aeb6 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -815,6 +815,8 @@ struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos, size_t len)
 		 */
 		if (off || len < unit) {
 			fgp |= FGP_DONTCACHE_EDGE;
+			if (iter->flags & IOMAP_DONTCACHE_DEFER)
+				fgp |= FGP_DONTCACHE_DEFER;
 			len = min_t(size_t, len, unit - off);
 		}
 	}
@@ -1322,6 +1324,8 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
 		iter.flags |= IOMAP_NOWAIT;
 	if (iocb->ki_flags & IOCB_DONTCACHE)
 		iter.flags |= IOMAP_DONTCACHE;
+	if (iocb->ki_flags & IOCB_DONTCACHE_DEFER)
+		iter.flags |= IOMAP_DONTCACHE_DEFER;
 
 	while ((ret = iomap_iter(&iter, ops)) > 0)
 		iter.status = iomap_write_iter(&iter, i, write_ops);
diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
index e1ea8392cf47..4f0343eeed1b 100644
--- a/fs/iomap/trace.h
+++ b/fs/iomap/trace.h
@@ -104,7 +104,8 @@ DEFINE_RANGE_EVENT(iomap_zero_iter);
 	{ IOMAP_UNSHARE,	"UNSHARE" }, \
 	{ IOMAP_DAX,		"DAX" }, \
 	{ IOMAP_ATOMIC,		"ATOMIC" }, \
-	{ IOMAP_DONTCACHE,	"DONTCACHE" }
+	{ IOMAP_DONTCACHE,	"DONTCACHE" }, \
+	{ IOMAP_DONTCACHE_DEFER, "DONTCACHE_DEFER" }
 
 #define IOMAP_F_FLAGS_STRINGS \
 	{ IOMAP_F_NEW,		"NEW" }, \
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f9d1e05e8ae6..ad181b6ce6b4 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -357,6 +357,11 @@ struct readahead_control;
 /* kiocb is a read or write operation submitted by fs/aio.c. */
 #define IOCB_AIO_RW		(1 << 22)
 #define IOCB_HAS_METADATA	(1 << 23)
+/*
+ * With IOCB_DONTCACHE: the caller completes the pages the write covers only
+ * partly itself, see filemap_dontcache_edge_complete().
+ */
+#define IOCB_DONTCACHE_DEFER	(1 << 24)
 
 /* for use in trace events */
 #define TRACE_IOCB_STRINGS \
@@ -374,7 +379,8 @@ struct readahead_control;
 	{ IOCB_NOIO,		"NOIO" }, \
 	{ IOCB_ALLOC_CACHE,	"ALLOC_CACHE" }, \
 	{ IOCB_AIO_RW,		"AIO_RW" }, \
-	{ IOCB_HAS_METADATA,	"AIO_HAS_METADATA" }
+	{ IOCB_HAS_METADATA,	"AIO_HAS_METADATA" }, \
+	{ IOCB_DONTCACHE_DEFER,	"DONTCACHE_DEFER" }
 
 struct kiocb {
 	struct file		*ki_filp;
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index bc7ae6327dbf..f3509d495a92 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -212,6 +212,7 @@ struct iomap_write_ops {
 #endif /* CONFIG_FS_DAX */
 #define IOMAP_ATOMIC		(1 << 9) /* torn-write protection */
 #define IOMAP_DONTCACHE		(1 << 10)
+#define IOMAP_DONTCACHE_DEFER	(1 << 11) /* see IOCB_DONTCACHE_DEFER */
 
 /*
  * Return the existing mapping at pos, or reserve space starting at pos for up
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 0c1d6bd09aa0..a0a67e4c940b 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -705,6 +705,9 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
  *   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_DONTCACHE_DEFER - With %FGP_DONTCACHE_EDGE: the caller completes the
+ *   page itself (filemap_dontcache_edge_complete()), so a folio found uptodate
+ *   is not marked either.
  * * %FGP_WRITEBEGIN - The flags to use in a filesystem write_begin()
  *   implementation.
  */
@@ -720,6 +723,7 @@ typedef unsigned int __bitwise fgf_t;
 #define FGP_STABLE		((__force fgf_t)0x00000080)
 #define FGP_DONTCACHE		((__force fgf_t)0x00000100)
 #define FGP_DONTCACHE_EDGE	((__force fgf_t)0x00000200)
+#define FGP_DONTCACHE_DEFER	((__force fgf_t)0x00000400)
 #define FGF_GET_ORDER(fgf)	(((__force unsigned)fgf) >> 26)	/* top 6 bits */
 
 #define FGP_WRITEBEGIN		(FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE)
@@ -1316,6 +1320,8 @@ void folio_end_writeback(struct folio *folio);
 void folio_end_writeback_no_dropbehind(struct folio *folio);
 void folio_end_dropbehind(struct folio *folio);
 void folio_mark_dropbehind(struct folio *folio);
+bool filemap_dontcache_edge_completes(struct address_space *mapping, loff_t pos);
+void filemap_dontcache_edge_complete(struct address_space *mapping, loff_t pos);
 void folio_wait_stable(struct folio *folio);
 void __folio_mark_dirty(struct folio *folio, struct address_space *, int warn);
 void folio_account_cleaned(struct folio *folio, struct bdi_writeback *wb);
diff --git a/mm/filemap.c b/mm/filemap.c
index 1494e42e609f..890913f38c65 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1695,6 +1695,72 @@ static bool filemap_dontcache_edge_unit(struct address_space *mapping,
 	return folio_nr_pages(folio) == mapping_min_folio_nrpages(mapping);
 }
 
+/**
+ * filemap_dontcache_edge_completes - will this write complete a shared page?
+ * @mapping: the file's mapping
+ * @pos: any byte offset within the page
+ *
+ * For an uncached writer that completes the pages it covers only partly
+ * itself (IOCB_DONTCACHE_DEFER).  Call right before writing such a page: it
+ * is in the page cache and uptodate when the neighbouring write has already
+ * written its part, and this write then completes it.  Two writes that
+ * arrive together may both find it absent; the page is then left cached,
+ * unmarked, until reclaim.
+ *
+ * Return: true if the caller is to call filemap_dontcache_edge_complete()
+ * once its write, including any sync of it, is done.
+ */
+bool filemap_dontcache_edge_completes(struct address_space *mapping, loff_t pos)
+{
+	struct folio *folio;
+	bool ret;
+
+	/* FGP_DONTCACHE: a plain lookup would clear a drop-behind mark. */
+	folio = __filemap_get_folio(mapping, pos >> PAGE_SHIFT, FGP_DONTCACHE, 0);
+	if (IS_ERR(folio))
+		return false;
+	ret = folio_test_uptodate(folio) &&
+	      filemap_dontcache_edge_unit(mapping, folio);
+	folio_put(folio);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(filemap_dontcache_edge_completes);
+
+/**
+ * filemap_dontcache_edge_complete - drop a shared page both writes have written
+ * @mapping: the file's mapping
+ * @pos: any byte offset within the page
+ *
+ * Mark the page drop-behind.  If it is already written back, drop it now;
+ * otherwise the writeback that cleans it drops it.  A dirty page marked here
+ * counts as DONTCACHE dirty and wakes the DONTCACHE writeback kick, so a
+ * caller that syncs its write calls this after the sync.  A folio larger
+ * than the smallest the mapping allows is left as it is: it holds more than
+ * the two writes' page, and it cannot be dropped in part.
+ */
+void filemap_dontcache_edge_complete(struct address_space *mapping, loff_t pos)
+{
+	struct folio *folio;
+
+	folio = __filemap_get_folio(mapping, pos >> PAGE_SHIFT,
+				    FGP_LOCK | FGP_DONTCACHE, 0);
+	if (IS_ERR(folio))
+		return;
+	if (filemap_dontcache_edge_unit(mapping, folio)) {
+		folio_mark_dropbehind(folio);
+		/*
+		 * Drop it now if it is already clean, with the lock still
+		 * held: folio_end_dropbehind() would take it again with a
+		 * trylock, and a folio another task locks in between would
+		 * be left for reclaim, no writeback being left to drop it.
+		 */
+		filemap_end_dropbehind(folio);
+	}
+	folio_unlock(folio);
+	folio_put(folio);
+}
+EXPORT_SYMBOL_GPL(filemap_dontcache_edge_complete);
+
 /**
  * folio_end_writeback_no_dropbehind - End writeback against a folio.
  * @folio: The folio.
@@ -2085,8 +2151,8 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
 	 * more than they wrote and cannot be dropped in part.
 	 */
 	if ((fgp_flags & FGP_DONTCACHE) && (fgp_flags & FGP_DONTCACHE_EDGE) &&
-	    (fgp_flags & FGP_LOCK) && folio_test_uptodate(folio) &&
-	    filemap_dontcache_edge_unit(mapping, folio))
+	    !(fgp_flags & FGP_DONTCACHE_DEFER) && (fgp_flags & FGP_LOCK) &&
+	    folio_test_uptodate(folio) && filemap_dontcache_edge_unit(mapping, folio))
 		folio_mark_dropbehind(folio);
 no_page:
 	if (!folio && (fgp_flags & FGP_CREAT)) {
-- 
2.52.0

[PATCH v8 7/7] NFSD: drop a shared boundary page only once both WRITEs have written it

[Mike Snitzer]

A buffered DONTCACHE segment of a direct-mode WRITE -- the misaligned
start or end of a split WRITE, or the whole of one too small to split --
covers its first and last page only partly when it does not start or end
on a page boundary.  Such a page is shared with the neighbouring WRITE,
which may arrive before, after or at the same time, from any client.

The page cache keeps a page an uncached write covers only partly for the
write that completes it ("mm, iomap: keep a partly covered page until an
uncached write completes it"), so the second WRITE does not have to read
it back from disk.  Left at that, the page cache marks the page drop-
behind while it is still dirty, which counts it as DONTCACHE dirty and
wakes the DONTCACHE writeback kick for every segment of a stable WRITE,
only to write back what NFSD's single fsync is about to write.

Issue the buffered segments IOCB_DONTCACHE_DEFER and mark the pages here
instead ("mm, iomap: let an uncached writer complete its shared pages
itself").  Immediately before writing a segment, NFSD asks
filemap_dontcache_edge_completes() whether each page it covers only
partly has already been written by the neighbouring WRITE; once the
whole WRITE, its fsync included, is complete, it calls
filemap_dontcache_edge_complete() for those pages, which drops a clean
page at once.  An UNSTABLE WRITE marks them dirty, and the writeback
that cleans them, by the flusher or for the client's COMMIT, drops them.

A page whose neighbouring WRITE never arrives, such as the last page of
the file, or arrives at the same time, so that neither finds the other's
part, stays cached until it is reclaimed.  Nothing changes when the
buffered segments are normal buffered I/O (direct_misaligned_dontcache
is N, or the file system lacks FOP_DONTCACHE); the page cache keeps
partly covered pages only for file systems that use iomap for buffered
writes.

Measured with 32 interleaved writers of 47008-byte records over an
emulated 4Kn NVMe with 4 KiB pages, NFSD DIRECT with
direct_misaligned_dontcache=Y, each run from a freshly made file system:
device reads per WRITE are 0.0001-0.0002 whether each record is one
FILE_SYNC WRITE, two sent one after the other, two sent together, or two
sent UNSTABLE, against 45144 reads for 45664 WRITEs with no shared-page
handling, and the DONTCACHE writeback kick does not run at all for the
FILE_SYNC WRITEs.

On a four-server pNFS flexfiles rig (x86_64, 4 KiB logical blocks), three
clients writing 47008-byte records for 240 s against 640 nfsd threads per
server, keeping the shared page took device reads from 0.014-0.015 per
record to 0.003, page-cache growth over the write phase from about
11 GiB to 2.9 GiB, and write throughput up by 2.4% to 4.9%, with read
throughput unchanged.  Keeping every boundary page cached instead, with
direct_misaligned_dontcache=N, grows the page cache by about 630 GiB over
the same runs.

Reported-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 .../filesystems/nfs/nfsd-io-modes.rst         | 17 ++++
 fs/nfsd/vfs.c                                 | 99 ++++++++++++++++---
 2 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/nfs/nfsd-io-modes.rst b/Documentation/filesystems/nfs/nfsd-io-modes.rst
index 0e69b5fdd675..9c663ce97c48 100644
--- a/Documentation/filesystems/nfs/nfsd-io-modes.rst
+++ b/Documentation/filesystems/nfs/nfsd-io-modes.rst
@@ -167,6 +167,23 @@ Misaligned WRITE:
     above. Set it to N for a workload that reads back or rewrites what
     it just wrote.
 
+    The page holding a start or end segment is shared with the
+    neighbouring WRITE, which may arrive before, after or at the same
+    time, from any client. So that the second WRITE to such a page does
+    not have to read it back from disk, the page cache keeps a page an
+    uncached write covers only partly, unmarked, for the write that
+    completes it. NFSD asks, immediately before writing a start or end
+    segment, whether the neighbouring WRITE has already written its
+    part; if it has, NFSD marks the page DONTCACHE once its own WRITE,
+    including its fsync, is complete, and the page is dropped. A page
+    whose neighbouring WRITE never arrives, or arrives at the same time,
+    stays cached until it is reclaimed. The first and last pages of a
+    WRITE that is not split are handled the same way.
+
+    None of this applies when the start and end segments are normal
+    buffered IO, or for a filesystem that does not use iomap for
+    buffered writes.
+
 Tracing:
     The nfsd_read_direct trace event marks a READ that NFSD issues as
     O_DIRECT. It records the offset and length the client requested.
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index ade00dabef18..f151289345c9 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1387,6 +1387,7 @@ static int wait_for_concurrent_writes(struct file *file)
 struct nfsd_write_dio_seg {
 	struct iov_iter			iter;
 	int				flags;
+	bool				claim;		/* completes shared pages */
 };
 
 static unsigned long
@@ -1406,6 +1407,65 @@ nfsd_write_dio_seg_init(struct nfsd_write_dio_seg *segment,
 		iov_iter_advance(&segment->iter, start);
 	iov_iter_truncate(&segment->iter, len);
 	segment->flags = iocb->ki_flags;
+	segment->claim = false;
+}
+
+/* The shared pages a segment completes, and how much of it was written. */
+struct nfsd_write_dio_hold {
+	loff_t	first, last;
+	bool	complete_first, complete_last;
+	size_t	written;
+};
+
+/*
+ * A page that a buffered DONTCACHE segment covers only partly is shared with
+ * the neighbouring WRITE.  The page cache keeps it, unmarked, for the write
+ * that completes it; the segment is issued IOCB_DONTCACHE_DEFER so that this
+ * WRITE, not the page cache, marks it, once the whole WRITE, its fsync
+ * included, is complete.  Ask whether the neighbour has already written its
+ * part immediately before writing @seg: asking earlier gives the neighbour
+ * time to write its part and find this WRITE's part missing.
+ */
+static void
+nfsd_write_dio_seg_hold(struct file *file, const struct nfsd_write_dio_seg *seg,
+			loff_t pos, size_t len, struct nfsd_write_dio_hold *hold)
+{
+	struct address_space *mapping = file->f_mapping;
+
+	hold->first = pos;
+	hold->last = pos + len - 1;
+	hold->complete_first = false;
+	hold->complete_last = false;
+	hold->written = 0;
+	if (seg->claim && len) {
+		bool one_page = (hold->first >> PAGE_SHIFT) ==
+				(hold->last >> PAGE_SHIFT);
+		bool head = hold->first & ~PAGE_MASK;
+		bool tail = (hold->last + 1) & ~PAGE_MASK;
+
+		/* Only a page the segment covers partly is shared. */
+		if (head || (one_page && tail))
+			hold->complete_first =
+				filemap_dontcache_edge_completes(mapping, hold->first);
+		if (!one_page && tail)
+			hold->complete_last =
+				filemap_dontcache_edge_completes(mapping, hold->last);
+	}
+}
+
+/* Drop the shared pages this WRITE was the second of two to write. */
+static void
+nfsd_write_dio_seg_release(struct file *file,
+			   const struct nfsd_write_dio_hold *hold)
+{
+	struct address_space *mapping = file->f_mapping;
+
+	if (hold->complete_first && hold->written)
+		filemap_dontcache_edge_complete(mapping, hold->first);
+	/* A short write did not reach the last page. */
+	if (hold->complete_last &&
+	    hold->written == hold->last - hold->first + 1)
+		filemap_dontcache_edge_complete(mapping, hold->last);
 }
 
 static unsigned int
@@ -1428,7 +1488,9 @@ nfsd_write_dio_iters_init(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	if (nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
 		dontcache_flags = IOCB_DONTCACHE;
-	buffered_flags = nfsd_direct_misaligned_dontcache ? dontcache_flags : 0;
+	/* NFSD marks their shared pages itself, see nfsd_write_dio_seg_hold() */
+	buffered_flags = nfsd_direct_misaligned_dontcache && dontcache_flags ?
+			 dontcache_flags | IOCB_DONTCACHE_DEFER : 0;
 
 	/*
 	 * Check if direct I/O is feasible for this write request.
@@ -1485,7 +1547,8 @@ nfsd_write_dio_iters_init(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (prefix) {
 		nfsd_write_dio_seg_init(&segments[nsegs], bvec,
 					nvecs, total, 0, prefix, iocb);
-		segments[nsegs++].flags |= buffered_flags;
+		segments[nsegs].flags |= buffered_flags;
+		segments[nsegs++].claim = !!buffered_flags;
 	}
 
 	nfsd_write_dio_seg_init(&segments[nsegs], bvec, nvecs,
@@ -1518,7 +1581,8 @@ nfsd_write_dio_iters_init(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	if (suffix) {
 		nfsd_write_dio_seg_init(&segments[nsegs], bvec, nvecs, total,
 					prefix + middle, suffix, iocb);
-		segments[nsegs++].flags |= buffered_flags;
+		segments[nsegs].flags |= buffered_flags;
+		segments[nsegs++].claim = !!buffered_flags;
 	}
 	return nsegs;
 
@@ -1527,6 +1591,7 @@ nfsd_write_dio_iters_init(struct svc_rqst *rqstp, struct svc_fh *fhp,
 	nfsd_write_dio_seg_init(&segments[0], bvec, nvecs, total, 0,
 				total, iocb);
 	segments[0].flags |= buffered_flags;
+	segments[0].claim = !!buffered_flags;
 	return 1;
 }
 
@@ -1535,12 +1600,13 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		  struct nfsd_file *nf, unsigned int nvecs,
 		  unsigned long *cnt, struct kiocb *kiocb)
 {
+	struct nfsd_write_dio_hold holds[3];
 	struct nfsd_write_dio_seg segments[3];
 	struct file *file = nf->nf_file;
 	loff_t start = kiocb->ki_pos;
-	unsigned int nsegs, i;
+	unsigned int nsegs, nheld = 0, i;
+	ssize_t host_err = 0;
 	int sync_flags;
-	ssize_t host_err;
 	size_t expected;
 
 	nsegs = nfsd_write_dio_iters_init(rqstp, fhp, nf, rqstp->rq_bvec,
@@ -1569,23 +1635,32 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
 					kiocb->ki_flags);
 
 		expected = iov_iter_count(&segments[i].iter);
-
+		nfsd_write_dio_seg_hold(file, &segments[i], kiocb->ki_pos,
+					expected, &holds[nheld]);
 		host_err = vfs_iocb_iter_write(file, kiocb, &segments[i].iter);
 		if (host_err < 0)
-			return host_err;
+			break;
+		holds[nheld++].written = host_err;
 		*cnt += host_err;
 		if (host_err < (ssize_t)expected)
 			break;	/* partial write */
 	}
 
-	if (sync_flags && *cnt) {
+	if (host_err >= 0 && sync_flags && *cnt)
 		host_err = vfs_fsync_range(file, start, start + *cnt - 1,
 					   !(sync_flags & IOCB_SYNC));
-		if (host_err < 0)
-			return host_err;
-	}
 
-	return 0;
+	/*
+	 * Mark the pages this WRITE shares with its neighbours for dropping
+	 * only once the WRITE is complete, including its fsync if it has one.
+	 * A page marked while still dirty is counted as DONTCACHE dirty and
+	 * wakes the DONTCACHE writeback kick; a page marked once clean is
+	 * simply dropped.
+	 */
+	for (i = 0; i < nheld; i++)
+		nfsd_write_dio_seg_release(file, &holds[i]);
+
+	return host_err < 0 ? host_err : 0;
 }
 
 /**
-- 
2.52.0

Re: [PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages

[Jeff Layton]

On Thu, 2026-10-22 at 22:45 -0400, Mike Snitzer wrote:
> Hi,
> 
> Please consider this v8 for the v7.5 development cycle, so that it can
> go upstream in the v7.5 merge window.
> 
> An uncached (RWF_DONTCACHE) buffered write drops each page it writes
> once that page is written back.  A page the write covers only partly is
> shared with the neighbouring write, and once the first of the two has
> been written back the second has to read it back from disk before it
> can write its part.  32 threads writing interleaved records to one file
> on XFS (emulated 4Kn NVMe, 4 KiB pages), before and after 5/7:
> 
>   record  pwritev2() flags          4 KiB device       MiB/s
>   bytes                             reads per write
>                                     before   after     before  after
>   47008   RWF_DONTCACHE             0.97     0.0005    831     2281
>   47008   RWF_DONTCACHE|RWF_DSYNC   0.99     0.0005    601     1227
>   6000    RWF_DONTCACHE             0.97     0.0001    121      488
>   6000    RWF_DONTCACHE|RWF_DSYNC   0.99     0.0002     90      206
> 
> What is left after is metadata: 49152-byte records, which cover only
> whole pages, read 0.0005 per write either way.  The test program and
> script that produced this table, and how to run them, are in:
> https://lore.kernel.org/linux-nfs/atqJBS8MlYfeUU-t@kernel.org/
> 
> 5/7 has the page cache keep such a page for the write that completes
> it.  6/7 lets a writer that syncs its own write, as NFSD does for a
> stable WRITE, complete those pages itself once the sync is done.  7/7
> is the NFSD change that uses it.  3/7 reports the DONTCACHE dirty count
> in the bdi debugfs stats, and 4/7 adds folio_mark_dropbehind(), which
> 5/7 and 6/7 use.
> 
> 1/7 and 2/7 are fixes the review of v7 turned up in code that is
> already queued or upstream, and each stands on its own:
> 
> - 1/7 fixes 80728157d1bd ("NFSD: cut a misaligned direct-mode WRITE at
>   page boundaries") in cel/nfsd-testing: where the mapping's smallest
>   folio is a block larger than a page, NFSD cut a WRITE's misaligned
>   ends inside a block, so the direct middle and a buffered end shared
>   it.  Chuck, maybe best to fold into 80728157d1bd ?
> - 2/7 fixes 88d6f128d06d ("mm: track DONTCACHE dirty pages per
>   bdi_writeback"), in v7.3: a cached lookup that clears the dropbehind
>   flag could take a folio off WB_DONTCACHE_DIRTY that was never counted
>   there, or leave one counted with nothing to uncount it.  It applies
>   to v7.3 as it is, if the mm side would rather take it alone.
> 
> The prerequisite NFSD changes are staged in cel/nfsd-testing; this
> series applies on top of it at 6d13b7f74eec ("NFSD: Map every NFS_ACLv2
> reply status into aclstat2"), and each patch builds with W=1 with no
> warnings.
> 
> 2/7 to 6/7 are mm, with small iomap changes in 5/7 and 6/7, and are
> Cc'd to linux-mm and linux-fsdevel.  If the mm and iomap maintainers
> are agreeable, their acks would let Chuck take the series through the
> nfsd tree.  But whatever approach the maintainers prefer for this kind
> of cross-subsystem merge is obviously fine.
> 
> Changes since v7, from Jeff's review and the sashiko findings he
> relayed:
> - 1/7 and 2/7 are new (above).
> - 3/7 (v7 1/5): a comment dropped; Jeff's Reviewed-by.
> - 4/7 (v7 2/5): folio_mark_dropbehind() counted a folio as DONTCACHE
>   dirty when it found PG_dirty set, but PG_dirty is set before
>   folio_account_dirtied() counts the folio, without the folio lock, so
>   a folio marked in between was counted twice.  It now sets the flag
>   and tests the dirty tag, which is set in the same i_pages section that
>   counts the folio, under the i_pages lock.
> - 5/7 (v7 3/5): the edge is the smallest folio the mapping allows, a
>   page or a block where blocks are larger, both in iomap's test and in
>   what is marked: a larger folio found there, left by readahead or
>   another buffered write, is no longer marked whole.
> - 6/7 (v7 4/5): an edge already clean is dropped before the folio lock
>   is released, not after with a trylock that could miss it; and only
>   an edge-sized folio, as in 5/7.
> - 7/7 (v7 5/5): unchanged.
> 
> Tested on XFS on an emulated 4Kn NVMe, 4 KiB pages, 32 threads of
> interleaved 47008-byte records (12288 bytes where noted), 4 KiB device
> reads per write, v7 against v8:
> 
>                                              v7        v8
>   RWF_DONTCACHE over large folios
>     readahead left                          0.94      0.0026
>   RWF_DONTCACHE, 16 KiB blocks              1.62      0.0014
>     12288-byte records                      2.89      0.0004
>   NFS WRITEs through NFSD DIRECT,
>     16 KiB blocks                           3.46      0.0016
> 
> (v7 in 128 MiB runs, v8 in 1 GiB runs; on 16 KiB blocks one block read
> back per write is 4.0 here.)  In every run of v8, BdiDontcacheDirty
> (3/7) was back at 0 after the final sync, including runs mixing
> uncached writes, cached writes and mmap stores on one file, and every
> record verified.
> 
> v7: https://lore.kernel.org/linux-nfs/20261020220346.28020-1-snitzer@kernel.org/
> v6: https://lore.kernel.org/linux-nfs/atbfEbgMU6UGkn2X@kernel.org/
> v5: https://lore.kernel.org/linux-nfs/20261019150725.1507-1-snitzer@kernel.org/
> v4: https://lore.kernel.org/linux-nfs/20261015233309.42999-1-snitzer@kernel.org/
> v3: https://lore.kernel.org/linux-nfs/20261001045502.48381-1-snitzer@kernel.org/
> v2: https://lore.kernel.org/linux-nfs/20260929231329.22018-1-snitzer@kernel.org/
> v1: https://lore.kernel.org/linux-nfs/20260929173423.16149-1-snitzer@kernel.org/
> 
> Thanks,
> Mike
> 
> Mike Snitzer (7):
>   NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio
>   mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there
>   mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats
>   mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it
>   mm, iomap: keep a partly covered page until an uncached write completes it
>   mm, iomap: let an uncached writer complete its shared pages itself
>   NFSD: drop a shared boundary page only once both WRITEs have written it
> 
>  .../filesystems/nfs/nfsd-io-modes.rst         |  17 ++
>  fs/iomap/buffered-io.c                        |  20 +-
>  fs/iomap/trace.h                              |   3 +-
>  fs/nfsd/vfs.c                                 | 113 ++++++++--
>  include/linux/fs.h                            |   8 +-
>  include/linux/iomap.h                         |   1 +
>  include/linux/pagemap.h                       |  12 ++
>  mm/backing-dev.c                              |   6 +
>  mm/filemap.c                                  | 193 ++++++++++++++++--
>  9 files changed, 338 insertions(+), 35 deletions(-)
> 
> 
> base-commit: 6d13b7f74eec16e08d5e7f772ee119432a14ef2c

It might not hurt to have your LLM simplify the patch descriptions
here. I find that my appetite for reading claude-ish walls of text has
waned in recent weeks. This skill helps a lot:

    https://github.com/danyuchn/asd-ste100-skill

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages

[Mike Snitzer]

On Fri, Oct 23, 2026 at 06:57:09AM -0400, Jeff Layton wrote:
> 
> It might not hurt to have your LLM simplify the patch descriptions
> here. I find that my appetite for reading claude-ish walls of text has
> waned in recent weeks. This skill helps a lot:
> 
>     https://github.com/danyuchn/asd-ste100-skill

Thanks, I've now trained my claude monkey with it. But I'm sure it'll
still throw feces periodically.

Also, I've reviewed the sashiko review of this v8, that you kindly
forwarded this morning, and 3 of the 7 issues it flagged were real.

I don't want to spam the lists more than I already have by immediately
posting v9... but it has needed changes, blech.  I'll at least wait to
see if Chuck picks up 1/7 to fold into the commit it fixes which is
already staged in cel/nfsd-testing (and if so I'll move Chuck and Jeff
to cc and upgrade -mm and -fsdevel to the To: for v9's posting...)

But here is the changelog for pending v9, in case it helps reviewers
side-step reporting something in v8 that is already addressed in v9:

Changes since v8, from the sashiko findings Jeff Layton relayed:
- 2/7 and 4/7: a folio is counted in WB_DONTCACHE_DIRTY only when both
  PG_dirty and its dirty tag are set.  v8 tested the tag alone.
  __folio_cancel_dirty() clears PG_dirty but leaves the tag, so a
  cancelled folio that stays cached kept a stale tag.  2/7 then
  uncounted such a folio twice, and 4/7 counted it and never uncounted
  it.
- 7/7: NFSD finds the edges a buffered segment shares by the mapping's
  smallest folio, as iomap does, not by page.  On 16 KiB blocks, a WRITE
  written as one buffered segment can have page-aligned ends inside a
  block.  Nothing completed those edges, and the folios stayed cached
  until reclaim.  NFSD also completes a segment's first edge only when
  the write reached the end of the segment's bytes in that unit.  A
  short write that stopped inside the unit had not written its part.
- 1/7 and 5/7: a paragraph each on a limit the review raised.  1/7
  cannot keep a larger cached folio, left by readahead, from spanning
  the cut.  5/7 marks the folio before iomap validates the iomap.  A
  stale iomap stopped by a fatal signal then leaves a mark on a folio
  this write did not complete.  Both cost one read-back and no data.
- 3/7 and 6/7: unchanged.

Longer form for the fixes that fell out from the v8 sashiko review:

2/7, the stale dirty tag: real.  __folio_cancel_dirty() uncounts a
dropbehind folio and clears PG_dirty, but leaves PAGECACHE_TAG_DIRTY
set.  Only writeback and truncation clear the tag, and writeback skips
a clean folio.  So a cancelled folio that stays cached keeps a stale
tag, and my clear path uncounted it a second time.  The same stale tag
made folio_mark_dropbehind() in 4/7 count a clean folio that nothing
would uncount.  v9 tests PG_dirty as well as the tag in both places.
A folio is counted only when both are set.  The race the tag closes
stays closed.  A dirtier that has set PG_dirty but not yet counted the
folio has not set the tag either.

7/7, the PAGE_SIZE edge tests: real.  On 16 KiB blocks, a WRITE written
as one buffered segment can have page-aligned ends inside a block.
iomap left those edges for NFSD to complete, and NFSD never asked about
them.  Those folios stayed cached until reclaim.  v9 finds the edges by the
mapping's smallest folio, as iomap does.  Measured on the loop device,
whose dma_alignment of 511 makes every TCP payload memory-misaligned.
NFSD then writes each WRITE as one buffered segment.  32 writers of
12288-byte records: v8 left all 32,736 pages of a 128 MiB file
resident.  v9 leaves 1,348 of 262,080 pages of a 1 GiB file, 0.5%.

7/7, the short write that completes the first edge: real, and the cost
is one read-back on the retry.  v9 completes the first edge only when
the write reached the end of the segment's bytes in that unit.

Re: [PATCH v8 1/7] NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio

[Chuck Lever]

On Thu, Oct 22, 2026, at 10:45 PM, Mike Snitzer wrote:
> nfsd_write_dio_iters_init() cuts a misaligned end of a direct-mode
> WRITE at a multiple of the larger of offset_align and PAGE_SIZE, so
> that the direct middle, which invalidates the page cache over its
> range, never shares a page with a buffered end.  But where the
> mapping's minimum folio order makes the smallest folio a block larger
> than a page, the page cache holds that block as one folio: an end cut
> on a page boundary inside a block leaves the middle and the buffered
> end in the same folio.  The middle's invalidation then drops the folio
> the end wrote, and whichever of the two neighbouring WRITEs comes second
> reads the block back.
>
> Cut a misaligned end at a multiple of the smallest folio the mapping
> allows as well.  Where that is a page, nothing changes.
>
> Fixes: 80728157d1bd ("NFSD: cut a misaligned direct-mode WRITE at page 
> boundaries")
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  fs/nfsd/vfs.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 9219f8eb386d..ade00dabef18 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1452,13 +1452,15 @@ nfsd_write_dio_iters_init(struct svc_rqst 
> *rqstp, struct svc_fh *fhp,
> 
>  	/*
>  	 * The direct middle invalidates the page cache over its range, so it
> -	 * must not share a page with a buffered end.  An aligned end needs no
> -	 * buffered segment; a misaligned one is cut on a page boundary even
> -	 * where the offset alignment is finer than a page.  A WRITE may end
> -	 * at OFFSET_MAX, so the prefix is sized as a distance rather than
> -	 * by rounding @offset up, which could wrap.
> +	 * must not share a folio with a buffered end.  An aligned end needs no
> +	 * buffered segment; a misaligned one is cut on a boundary of the
> +	 * smallest folio the mapping allows, a page or, where the block is
> +	 * larger, a block, even where the offset alignment is finer.  A WRITE
> +	 * may end at OFFSET_MAX, so the prefix is sized as a distance rather
> +	 * than by rounding @offset up, which could wrap.
>  	 */
> -	cut = max_t(u32, offset_align, PAGE_SIZE);
> +	cut = max3(offset_align, PAGE_SIZE,
> +		   mapping_min_folio_nrbytes(nf->nf_file->f_mapping));
>  	prefix_end = offset;
>  	if (!IS_ALIGNED(offset, offset_align))
>  		prefix_end += min_t(loff_t, cut - (offset & (cut - 1)), total);
> -- 
> 2.52.0

PAGE_SIZE looks dead in the max3(). mapping_min_folio_nrbytes() is always
at least PAGE_SIZE, so the cut is just max(offset_align, min folio). The
new commit message and comment both describe three independent bounds, but
the accurate statement is two: the DIO offset alignment and the mapping's
smallest folio.

Also I would like to keep the helper's unsigned long type instead of
narrowing to u32.

Post an updated version of the v7 patch so we can test and review that
as a single change. This will also archive that revision in the lore
archive. This will likely result in merge conflicts with subsequent
patches in the series. If those are just comment conflicts I can fix
those up when I apply the revised version.

Do not include the new revision of this patch in v9. v9 is now patches
destined for the MM tree(s) while this patch goes to NFSD.


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH v8 1/7] NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio

[Mike Snitzer]

On Fri, Oct 23, 2026 at 01:45:21PM -0400, Chuck Lever wrote:
> 
> 
> On Thu, Oct 22, 2026, at 10:45 PM, Mike Snitzer wrote:
> > nfsd_write_dio_iters_init() cuts a misaligned end of a direct-mode
> > WRITE at a multiple of the larger of offset_align and PAGE_SIZE, so
> > that the direct middle, which invalidates the page cache over its
> > range, never shares a page with a buffered end.  But where the
> > mapping's minimum folio order makes the smallest folio a block larger
> > than a page, the page cache holds that block as one folio: an end cut
> > on a page boundary inside a block leaves the middle and the buffered
> > end in the same folio.  The middle's invalidation then drops the folio
> > the end wrote, and whichever of the two neighbouring WRITEs comes second
> > reads the block back.
> >
> > Cut a misaligned end at a multiple of the smallest folio the mapping
> > allows as well.  Where that is a page, nothing changes.
> >
> > Fixes: 80728157d1bd ("NFSD: cut a misaligned direct-mode WRITE at page 
> > boundaries")
> > Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > ---
> >  fs/nfsd/vfs.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 9219f8eb386d..ade00dabef18 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1452,13 +1452,15 @@ nfsd_write_dio_iters_init(struct svc_rqst 
> > *rqstp, struct svc_fh *fhp,
> > 
> >  	/*
> >  	 * The direct middle invalidates the page cache over its range, so it
> > -	 * must not share a page with a buffered end.  An aligned end needs no
> > -	 * buffered segment; a misaligned one is cut on a page boundary even
> > -	 * where the offset alignment is finer than a page.  A WRITE may end
> > -	 * at OFFSET_MAX, so the prefix is sized as a distance rather than
> > -	 * by rounding @offset up, which could wrap.
> > +	 * must not share a folio with a buffered end.  An aligned end needs no
> > +	 * buffered segment; a misaligned one is cut on a boundary of the
> > +	 * smallest folio the mapping allows, a page or, where the block is
> > +	 * larger, a block, even where the offset alignment is finer.  A WRITE
> > +	 * may end at OFFSET_MAX, so the prefix is sized as a distance rather
> > +	 * than by rounding @offset up, which could wrap.
> >  	 */
> > -	cut = max_t(u32, offset_align, PAGE_SIZE);
> > +	cut = max3(offset_align, PAGE_SIZE,
> > +		   mapping_min_folio_nrbytes(nf->nf_file->f_mapping));
> >  	prefix_end = offset;
> >  	if (!IS_ALIGNED(offset, offset_align))
> >  		prefix_end += min_t(loff_t, cut - (offset & (cut - 1)), total);
> > -- 
> > 2.52.0
> 
> PAGE_SIZE looks dead in the max3(). mapping_min_folio_nrbytes() is always
> at least PAGE_SIZE, so the cut is just max(offset_align, min folio). The
> new commit message and comment both describe three independent bounds, but
> the accurate statement is two: the DIO offset alignment and the mapping's
> smallest folio.
> 
> Also I would like to keep the helper's unsigned long type instead of
> narrowing to u32.
> 
> Post an updated version of the v7 patch so we can test and review that
> as a single change. This will also archive that revision in the lore
> archive. This will likely result in merge conflicts with subsequent
> patches in the series. If those are just comment conflicts I can fix
> those up when I apply the revised version.
> 
> Do not include the new revision of this patch in v9. v9 is now patches
> destined for the MM tree(s) while this patch goes to NFSD.

v7 didn't have this 1/7 patch.  But seems you're saying the fix for
nfsd-testing commit 80728157d1bd (which started as v4 1/10) would be
to have max_t() use mapping_min_folio_nrbytes(nf->nf_file->f_mapping)
instead of PAGE_SIZE?

I think you're saying post a revised version of commit 80728157d1bd as
a standalone NFSD patch?  I can check if it causes the other patches
you've staged to have conflicts (but like you say, should be simple to
resolve).

Thanks,
Mike

Re: [PATCH v8 2/7] mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: double-decrement of WB_DONTCACHE_DIRTY in filemap_clear_dropbehind()
--

commit 4fb0b9f6bc322dc01cdee84f7076d22537c5bd27
Author: Mike Snitzer <snitzer@kernel.org>

mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there

This commit introduces filemap_clear_dropbehind() to clear PG_dropbehind
and decrement WB_DONTCACHE_DIRTY under the folio lock and i_pages lock
when a cached lookup without FGP_DONTCACHE finds a dropbehind folio.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index 00fd89cf6f550..1cc28224d547a 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1927,6 +1927,45 @@ void *filemap_get_entry(struct address_space *mapping, pgoff_t index)
>  	return folio;
>  }
>  
> +/*
> + * Clear the uncached flag of a folio a cached lookup found, and take the
> + * folio off WB_DONTCACHE_DIRTY if it is counted there.
> + *
> + * folio_account_dirtied() counts a dropbehind folio under the i_pages lock,
> + * in the section that sets its dirty tag, after PG_dirty has been set
> + * without the folio lock, and every path that uncounts a dirty folio holds
> + * the folio lock.  So with the folio locked and the i_pages lock held, the
> + * dirty tag, not PG_dirty, says whether the folio is counted: a dirtier past
> + * folio_test_set_dirty() but not yet counting finds the flag clear and does
> + * not count it.  A folio another task has locked keeps the flag, as it
> + * would have had this lookup not run.
> + */
> +static void filemap_clear_dropbehind(struct address_space *mapping,
> +				     struct folio *folio, bool locked)
> +{
> +	unsigned long flags;
> +
> +	if (!locked) {
> +		if (!folio_trylock(folio))
> +			return;
> +		if (unlikely(folio->mapping != mapping))
> +			goto out;
> +	}
> +	if (!mapping_can_writeback(mapping)) {
> +		folio_clear_dropbehind(folio);
> +		goto out;
> +	}
> +	xa_lock_irqsave(&mapping->i_pages, flags);
> +	if (folio_test_clear_dropbehind(folio) &&
> +	    xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY))
> +		wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY,
> +			    -folio_nr_pages(folio));
> +	xa_unlock_irqrestore(&mapping->i_pages, flags);

[Severity: Medium]
Can testing only PAGECACHE_TAG_DIRTY without folio_test_dirty(folio) in
filemap_clear_dropbehind() decrement WB_DONTCACHE_DIRTY a second time for
a folio that was already uncounted?

In fs/fs-writeback.c:inode_do_switch_wbs(), the PAGECACHE_TAG_DIRTY walk
checks folio_test_dirty(folio) before moving WB_RECLAIMABLE and
WB_DONTCACHE_DIRTY because PAGECACHE_TAG_DIRTY remains set in
mapping->i_pages after several paths clear PG_dirty and uncount the folio:

	xas_for_each_marked(&xas, folio, ULONG_MAX, PAGECACHE_TAG_DIRTY) {
		if (folio_test_dirty(folio)) {
			long nr = folio_nr_pages(folio);
			wb_stat_mod(old_wb, WB_RECLAIMABLE, -nr);
			wb_stat_mod(new_wb, WB_RECLAIMABLE, nr);
			if (folio_test_dropbehind(folio)) {
				wb_stat_mod(old_wb, WB_DONTCACHE_DIRTY, -nr);
				wb_stat_mod(new_wb, WB_DONTCACHE_DIRTY, nr);
			}
		}
	}

For example, during writeback,
writeback_iter()->writeback_get_folio()->folio_prepare_writeback() calls
folio_clear_dirty_for_io(), which clears PG_dirty and subtracts the folio
from WB_DONTCACHE_DIRTY while leaving PAGECACHE_TAG_DIRTY set in
mapping->i_pages (normally cleared later by folio_start_writeback()):

mm/page-writeback.c:folio_clear_dirty_for_io() {
	...
	if (folio_test_clear_dirty(folio)) {
		long nr = folio_nr_pages(folio);
		lruvec_stat_mod_folio(folio, NR_FILE_DIRTY, -nr);
		if (folio_test_dropbehind(folio))
			wb_stat_mod(wb, WB_DONTCACHE_DIRTY, -nr);
	...
}

If writeback races with truncate_setsize() lowering i_size, writeback
handlers such as iomap_writepages()->iomap_writeback_folio() see that the
folio is at or past i_size via iomap_writeback_handle_eof() and return
early without calling folio_start_writeback(). The folio is then unlocked
in mapping->i_pages with PG_dirty clear, PG_dropbehind still set, and
PAGECACHE_TAG_DIRTY still set. When truncate_inode_pages_range() or
another non-FGP_DONTCACHE caller subsequently looks up the folio via
__filemap_get_folio_mpol(), filemap_clear_dropbehind() will see
PAGECACHE_TAG_DIRTY set and subtract the folio from WB_DONTCACHE_DIRTY a
second time.

Similarly, mm/page-writeback.c:__folio_cancel_dirty() clears PG_dirty and
calls folio_account_cleaned(), subtracting WB_DONTCACHE_DIRTY without
clearing PAGECACHE_TAG_DIRTY or PG_dropbehind:

mm/page-writeback.c:__folio_cancel_dirty() {
	if (mapping_can_writeback(mapping)) {
		...
		if (folio_test_clear_dirty(folio))
			folio_account_cleaned(folio, wb);
		...
	} else {
		folio_clear_dirty(folio);
	}
}

When folio_cancel_dirty() is called on a folio that stays in the page
cache (for instance via filemap_release_folio()->try_to_free_buffers()
when reclaim or migration fails to remove the folio, or during partial
folio invalidation), PAGECACHE_TAG_DIRTY remains set on the clean
dropbehind folio. Furthermore, if such a folio with a lingering
PAGECACHE_TAG_DIRTY mark is later dirtied without the folio lock via
filemap_dirty_folio(), both PG_dirty and PAGECACHE_TAG_DIRTY are set
before __folio_mark_dirty() takes the i_pages lock to call
folio_account_dirtied().

Should filemap_clear_dropbehind() also require folio_test_dirty(folio), or
should paths that clear PG_dirty and uncount WB_DONTCACHE_DIRTY without
starting writeback also clear PAGECACHE_TAG_DIRTY (or PG_dropbehind)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023024548.71123-1-snitzer@kernel.org?part=2

Re: [PATCH v8 4/7] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: folio_mark_dropbehind() miscounts clean folios with stale dirty tag
--

commit 4468ce091bf80c4838cca1c8c9c9c92e78ab5c97
Author: Mike Snitzer <snitzer@kernel.org>

mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it

This commit adds folio_mark_dropbehind() to set PG_dropbehind on an existing
locked folio and account it in WB_DONTCACHE_DIRTY when the folio's
PAGECACHE_TAG_DIRTY mark is set in mapping->i_pages.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index 1cc28224d547a..0c76ee5750b3b 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1644,6 +1644,45 @@ void folio_end_dropbehind(struct folio *folio)
>  }
>  EXPORT_SYMBOL_GPL(folio_end_dropbehind);
>  
> +/**
> + * folio_mark_dropbehind - mark an existing folio dropbehind
> + * @folio: the folio, locked by the caller
> + *
> + * Mark @folio so that the writeback that cleans it drops it.  A folio
> + * already counted as dirty is added to WB_DONTCACHE_DIRTY, as
> + * folio_account_dirtied() would have added it had it been marked when
> + * dirtied, so that the clean that subtracts it leaves the counter balanced.
> + *
> + * folio_account_dirtied() counts a folio under the i_pages lock, in the
> + * section that sets its dirty tag, after PG_dirty has been set without the
> + * folio lock, and every path that uncounts a dirty folio holds the folio
> + * lock.  So with the folio locked and the i_pages lock held, the dirty tag,
> + * not PG_dirty, says whether the folio has been counted: a dirtier past
> + * folio_test_set_dirty() but not yet counting finds the flag set and adds
> + * the folio itself.
> + */
> +void folio_mark_dropbehind(struct folio *folio)
> +{
> +	struct address_space *mapping = folio->mapping;
> +	unsigned long flags;
> +
> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> +
> +	if (folio_test_dropbehind(folio))
> +		return;
> +	if (!mapping || !mapping_can_writeback(mapping)) {
> +		folio_set_dropbehind(folio);
> +		return;
> +	}
> +	xa_lock_irqsave(&mapping->i_pages, flags);
> +	folio_set_dropbehind(folio);
> +	if (xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY))
> +		wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY,
> +			    folio_nr_pages(folio));
> +	xa_unlock_irqrestore(&mapping->i_pages, flags);

[Severity: Medium]
Can folio_mark_dropbehind() miscount a clean folio in WB_DONTCACHE_DIRTY when
a lingering PAGECACHE_TAG_DIRTY mark remains set in mapping->i_pages after
PG_dirty was cleared?

In fs/fs-writeback.c, inode_do_switch_wbs() notes that PAGECACHE_TAG_DIRTY
points to possibly dirty folios and therefore checks both PAGECACHE_TAG_DIRTY
and folio_test_dirty(folio) before transferring WB_RECLAIMABLE and
WB_DONTCACHE_DIRTY:

	xas_for_each_marked(&xas, folio, ULONG_MAX, PAGECACHE_TAG_DIRTY) {
		if (folio_test_dirty(folio)) {
			long nr = folio_nr_pages(folio);
			wb_stat_mod(old_wb, WB_RECLAIMABLE, -nr);
			wb_stat_mod(new_wb, WB_RECLAIMABLE, nr);
			if (folio_test_dropbehind(folio)) {
				wb_stat_mod(old_wb, WB_DONTCACHE_DIRTY, -nr);
				wb_stat_mod(new_wb, WB_DONTCACHE_DIRTY, nr);
			}
		}
	}

Neither of the two dirty-uncounting functions in mm/page-writeback.c clears
PAGECACHE_TAG_DIRTY in mapping->i_pages:

1. __folio_cancel_dirty() clears PG_dirty via folio_test_clear_dirty(folio)
   and calls folio_account_cleaned(), leaving PAGECACHE_TAG_DIRTY set in
   mapping->i_pages:

	if (folio_test_clear_dirty(folio))
		folio_account_cleaned(folio, wb);

   Callers such as try_to_free_buffers() (for example via
   block_invalidate_folio() or filemap_release_folio() when
   __remove_mapping() fails), nfs_wb_folio_cancel(),
   netfs_invalidate_folio(), and orangefs_invalidate_folio() can leave the
   folio in mapping->i_pages with PG_dirty cleared and PAGECACHE_TAG_DIRTY
   still set.

2. folio_clear_dirty_for_io() clears PG_dirty and subtracts
   WB_DONTCACHE_DIRTY while leaving PAGECACHE_TAG_DIRTY set for
   __folio_start_writeback() to clear:

	if (folio_test_clear_dirty(folio)) {
		long nr = folio_nr_pages(folio);
		lruvec_stat_mod_folio(folio, NR_FILE_DIRTY, -nr);
		if (folio_test_dropbehind(folio))
			wb_stat_mod(wb, WB_DONTCACHE_DIRTY, -nr);
		...
	}

   Paths that call folio_clear_dirty_for_io() and unlock the folio without
   calling folio_start_writeback(), such as iomap_writeback_folio() when
   iomap_writeback_handle_eof() returns false at or beyond i_size,
   block_write_full_folio(), mpage_release_unused_pages(),
   f2fs_write_single_data_page(), ceph_writepages_start(), and
   btrfs_folio_clear_fixup_dirty(), also leave a clean folio in
   mapping->i_pages with PAGECACHE_TAG_DIRTY set. When writeback later visits
   such a folio in writeback_get_folio() -> folio_prepare_writeback(), it
   returns false on !folio_test_dirty(folio) without clearing
   PAGECACHE_TAG_DIRTY.

If folio_mark_dropbehind() is called on a locked folio in that state,
xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY) returns
true even though the folio is clean and not counted as dirty, so
WB_DONTCACHE_DIRTY is incremented by folio_nr_pages(folio).

If the caller then dirties the folio via folio_mark_dirty(),
filemap_dirty_folio() transitions PG_dirty from 0 to 1 and calls
__folio_mark_dirty() -> folio_account_dirtied(), which sees PG_dropbehind set
and adds folio_nr_pages(folio) to WB_DONTCACHE_DIRTY a second time, while
cleaning the folio later only subtracts it once. Or if the folio is evicted
or truncated without being dirtied again, neither folio_clear_dirty_for_io()
nor __folio_cancel_dirty() ever subtracts it because folio_test_clear_dirty()
is false, leaving WB_DONTCACHE_DIRTY permanently elevated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023024548.71123-1-snitzer@kernel.org?part=4

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mm, iomap: missing dropbehind in __filemap_get_folio_mpol() for sub-page blocks
- [Medium] mm/filemap: WB_DONTCACHE_DIRTY imbalance in folio_mark_dropbehind()
--

commit e20189c53d3ac3e13083b73f222d81bf9ea44e23
Author: Mike Snitzer <snitzer@kernel.org>

mm, iomap: keep a partly covered page until an uncached write completes it

This commit introduces FGP_DONTCACHE_EDGE so that uncached buffered writes
covering only part of a minimum-sized folio leave a newly created folio
without PG_dropbehind, and instead mark PG_dropbehind when a subsequent
neighbouring uncached write finds the folio uptodate and completes it.

> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 0a5ebfda90f12..86f884c9f8f22 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -802,8 +802,22 @@ 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) {
> +		size_t unit = mapping_min_folio_nrbytes(iter->inode->i_mapping);
> +		size_t off = pos & (unit - 1);
> +
>  		fgp |= FGP_DONTCACHE;
> +		/*
> +		 * The smallest folio the mapping allows, a page or a block
> +		 * where the block is larger, that this write covers only
> +		 * partly is shared with a neighbouring write: get it as just
> +		 * that, which is kept until the write that completes it.
> +		 */
> +		if (off || len < unit) {
> +			fgp |= FGP_DONTCACHE_EDGE;
> +			len = min_t(size_t, len, unit - off);
> +		}
> +	}

[Severity: Medium]
When the filesystem block size is smaller than PAGE_SIZE (for example, 4 KiB
blocks on a 16 KiB or 64 KiB page kernel, or sub-page blocks on a 4 KiB page
kernel), can folios written with RWF_DONTCACHE in sub-page chunks miss
getting marked with PG_dropbehind and remain in the page cache until memory
reclaim?

In iomap_get_folio(), unit is mapping_min_folio_nrbytes(), which is at least
PAGE_SIZE. Any uncached write with off != 0 or len < unit sets
FGP_DONTCACHE_EDGE, so __filemap_get_folio_mpol() allocates the folio
without setting PG_dropbehind.

When i_blocksize(inode) < PAGE_SIZE, __iomap_write_begin() attaches struct
iomap_folio_state and only zeroes or reads blocks outside the block-aligned
range being written. Then __iomap_write_end() calls
iomap_set_range_uptodate():

fs/iomap/buffered-io.c:iomap_set_range_uptodate() {
	if (ifs) {
		spin_lock_irqsave(&ifs->state_lock, flags);
		mark_uptodate = ifs_set_range_uptodate(folio, ifs, off, len) &&
				!ifs->read_bytes_pending;
		spin_unlock_irqrestore(&ifs->state_lock, flags);
	}

	if (mark_uptodate)
		folio_mark_uptodate(folio);
}

Whenever at least one sub-page block in the folio remains unwritten after
the first write, ifs_set_range_uptodate() returns false and PG_uptodate
stays clear on the folio.

When the completing RWF_DONTCACHE write arrives for the remaining block(s)
of that folio, iomap_write_begin() -> iomap_get_folio() ->
__filemap_get_folio_mpol() (and filemap_dontcache_edge_completes() in the
subsequent commit "mm, iomap: let an uncached writer complete its shared
pages itself") checks folio_test_uptodate(folio) before
__iomap_write_begin() and __iomap_write_end() run for the completing write.
Because the blocks about to be written by the completing write are not yet
uptodate in ifs->state, folio_test_uptodate(folio) is still false when the
folio is looked up, so folio_mark_dropbehind(folio) is skipped before
__iomap_write_end() finally sets PG_uptodate.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index 0c76ee5750b3b..1494e42e609fa 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c

[ ... ]

> @@ -2063,6 +2075,19 @@ 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.
> +	 * Not a folio larger than the edge the two writes share, which holds
> +	 * more than they wrote and cannot be dropped in part.
> +	 */
> +	if ((fgp_flags & FGP_DONTCACHE) && (fgp_flags & FGP_DONTCACHE_EDGE) &&
> +	    (fgp_flags & FGP_LOCK) && folio_test_uptodate(folio) &&
> +	    filemap_dontcache_edge_unit(mapping, folio))
> +		folio_mark_dropbehind(folio);

[Severity: Medium]
Can calling folio_mark_dropbehind() here before the caller dirties the folio
cause an imbalance in WB_DONTCACHE_DIRTY when PAGECACHE_TAG_DIRTY is still
set in mapping->i_pages but PG_dirty has already been cleared on the folio?

In folio_mark_dropbehind() (and similarly in filemap_clear_dropbehind()),
the WB_DONTCACHE_DIRTY adjustment checks PAGECACHE_TAG_DIRTY without checking
folio_test_dirty(folio):

mm/filemap.c:folio_mark_dropbehind() {
	...
	xa_lock_irqsave(&mapping->i_pages, flags);
	folio_set_dropbehind(folio);
	if (xa_get_mark(&mapping->i_pages, folio->index, PAGECACHE_TAG_DIRTY))
		wb_stat_mod(inode_to_wb(mapping->host), WB_DONTCACHE_DIRTY,
			    folio_nr_pages(folio));
	xa_unlock_irqrestore(&mapping->i_pages, flags);
}

By contrast, inode_do_switch_wbs() checks both PAGECACHE_TAG_DIRTY and
folio_test_dirty(folio) under mapping->i_pages.xa_lock, because the paths
that uncount a dirty folio under the folio lock --
folio_clear_dirty_for_io() and __folio_cancel_dirty() ->
folio_account_cleaned() -- clear PG_dirty and decrement WB_DONTCACHE_DIRTY
before PAGECACHE_TAG_DIRTY is cleared.

A folio can be unlocked while remaining resident and uptodate in
mapping->i_pages with PG_dirty == 0 and PAGECACHE_TAG_DIRTY == 1:

1. In iomap_writepages() -> writeback_get_folio() ->
   folio_prepare_writeback(), folio_clear_dirty_for_io(folio) clears
   PG_dirty and decrements WB_DONTCACHE_DIRTY. Then iomap_writeback_folio()
   checks iomap_writeback_handle_eof(); if the folio is at or beyond i_size
   (for example, dirtied at i_size by xfs_zero_range() -> iomap_zero_range()
   before i_size is updated, or raced with truncate_setsize()),
   iomap_writeback_folio() returns 0 without calling
   folio_start_writeback(), unlocking the folio with PG_dirty == 0 and
   PAGECACHE_TAG_DIRTY == 1.

2. On block device mappings using iomap buffered writes with buffer heads
   (block/fops.c sets FOP_DONTCACHE and IOMAP_F_BUFFER_HEAD), if a dirty
   folio's buffers are cleaned and try_to_free_buffers() runs,
   folio_cancel_dirty() clears PG_dirty and decrements WB_DONTCACHE_DIRTY
   while leaving PAGECACHE_TAG_DIRTY set.

If an uncached edge write then looks up such a folio (PG_dirty == 0,
PAGECACHE_TAG_DIRTY == 1, PG_dropbehind == 0) in __filemap_get_folio_mpol(),
folio_mark_dropbehind() sees PAGECACHE_TAG_DIRTY and increments
WB_DONTCACHE_DIRTY. When iomap_write_end() subsequently dirties the folio via
filemap_dirty_folio() or block_dirty_folio(), folio_test_set_dirty(folio)
transitions PG_dirty from 0 to 1 and folio_account_dirtied() sees
PG_dropbehind set, incrementing WB_DONTCACHE_DIRTY a second time for the
same folio (and conversely filemap_clear_dropbehind() would decrement
WB_DONTCACHE_DIRTY a second time if called on a dropbehind folio in that
state).

Should folio_mark_dropbehind() and filemap_clear_dropbehind() also verify
folio_test_dirty(folio) alongside PAGECACHE_TAG_DIRTY under
mapping->i_pages.xa_lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023024548.71123-1-snitzer@kernel.org?part=5

Re: [PATCH v8 6/7] mm, iomap: let an uncached writer complete its shared pages itself

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] mm: filemap_dontcache_edge_completes() misses partially uptodate iomap folios
- [Medium] mm: filemap_dontcache_edge_complete() misses writeback kick and racing bio drop
- [Medium] mm: WB_DONTCACHE_DIRTY leak in filemap_dontcache_edge_complete()
--

commit 7d2a5aea41a384cd5ec593563dbc8bd144b6bcce
Author: Mike Snitzer <snitzer@kernel.org>

mm, iomap: let an uncached writer complete its shared pages itself

This commit introduces IOCB_DONTCACHE_DEFER, IOMAP_DONTCACHE_DEFER, and
FGP_DONTCACHE_DEFER along with filemap_dontcache_edge_completes() and
filemap_dontcache_edge_complete() so that an uncached writer that syncs
its own writes can defer marking and dropping partially covered edge
folios until after the write and sync complete.

> diff --git a/mm/filemap.c b/mm/filemap.c
> index 1494e42e609fa..890913f38c65f 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1695,6 +1695,72 @@ static bool filemap_dontcache_edge_unit(struct address_space *mapping,
>  	return folio_nr_pages(folio) == mapping_min_folio_nrpages(mapping);
>  }
>  
> +/**
> + * filemap_dontcache_edge_completes - will this write complete a shared page?
> + * @mapping: the file's mapping
> + * @pos: any byte offset within the page
> + *
> + * For an uncached writer that completes the pages it covers only partly
> + * itself (IOCB_DONTCACHE_DEFER).  Call right before writing such a page: it
> + * is in the page cache and uptodate when the neighbouring write has already
> + * written its part, and this write then completes it.  Two writes that
> + * arrive together may both find it absent; the page is then left cached,
> + * unmarked, until reclaim.
> + *
> + * Return: true if the caller is to call filemap_dontcache_edge_complete()
> + * once its write, including any sync of it, is done.
> + */
> +bool filemap_dontcache_edge_completes(struct address_space *mapping, loff_t pos)
> +{
> +	struct folio *folio;
> +	bool ret;
> +
> +	/* FGP_DONTCACHE: a plain lookup would clear a drop-behind mark. */
> +	folio = __filemap_get_folio(mapping, pos >> PAGE_SHIFT, FGP_DONTCACHE, 0);
> +	if (IS_ERR(folio))
> +		return false;
> +	ret = folio_test_uptodate(folio) &&
> +	      filemap_dontcache_edge_unit(mapping, folio);
> +	folio_put(folio);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(filemap_dontcache_edge_completes);

[Severity: Medium]
What happens here (and in __filemap_get_folio_mpol()) on iomap filesystems
when the filesystem block size is smaller than the minimum folio size, i.e.
i_blocksize(inode) < PAGE_SIZE (for example, 4 KiB XFS blocks on a 16 KiB or
64 KiB PAGE_SIZE kernel, or sub-page block sizes on a 4 KiB PAGE_SIZE
kernel)?

When the first partial uncached write allocates the shared folio via
iomap_get_folio(), i_blocks_per_folio(inode, folio) is greater than 1, so
__iomap_write_begin() and __iomap_write_end() attach a struct
iomap_folio_state (ifs) and only mark the blocks within
[round_down(pos, block_size), round_up(pos + len, block_size)) uptodate in
ifs->state:

fs/iomap/buffered-io.c:iomap_set_range_uptodate() {
		mark_uptodate = ifs_set_range_uptodate(folio, ifs, off, len) &&
				!ifs->read_bytes_pending;
		spin_unlock_irqrestore(&ifs->state_lock, flags);
	}

	if (mark_uptodate)
		folio_mark_uptodate(folio);
}

Because the remaining blocks in the folio are not uptodate yet,
folio_mark_uptodate() is not called after the first partial write.

When the neighbouring write arrives to write the rest of the page and calls
filemap_dontcache_edge_completes() before writing (or looks up the folio in
__filemap_get_folio_mpol() before __iomap_write_end() runs),
folio_test_uptodate(folio) is still false.  Won't
filemap_dontcache_edge_completes() return false and leave the completed edge
folio cached until reclaim?

> +
> +/**
> + * filemap_dontcache_edge_complete - drop a shared page both writes have written
> + * @mapping: the file's mapping
> + * @pos: any byte offset within the page
> + *
> + * Mark the page drop-behind.  If it is already written back, drop it now;
> + * otherwise the writeback that cleans it drops it.  A dirty page marked here
> + * counts as DONTCACHE dirty and wakes the DONTCACHE writeback kick, so a
> + * caller that syncs its write calls this after the sync.  A folio larger
> + * than the smallest the mapping allows is left as it is: it holds more than
> + * the two writes' page, and it cannot be dropped in part.
> + */
> +void filemap_dontcache_edge_complete(struct address_space *mapping, loff_t pos)
> +{
> +	struct folio *folio;
> +
> +	folio = __filemap_get_folio(mapping, pos >> PAGE_SHIFT,
> +				    FGP_LOCK | FGP_DONTCACHE, 0);
> +	if (IS_ERR(folio))
> +		return;
> +	if (filemap_dontcache_edge_unit(mapping, folio)) {
> +		folio_mark_dropbehind(folio);
> +		/*
> +		 * Drop it now if it is already clean, with the lock still
> +		 * held: folio_end_dropbehind() would take it again with a
> +		 * trylock, and a folio another task locks in between would
> +		 * be left for reclaim, no writeback being left to drop it.
> +		 */
> +		filemap_end_dropbehind(folio);
> +	}
> +	folio_unlock(folio);
> +	folio_put(folio);
> +}
> +EXPORT_SYMBOL_GPL(filemap_dontcache_edge_complete);

[Severity: Medium]
There appear to be a few issues in filemap_dontcache_edge_complete() with
how writeback kicking and in-flight writeback are handled:

1. The kernel-doc states that a dirty page marked here counts as DONTCACHE
dirty and wakes the DONTCACHE writeback kick, and that an uncached writer
using IOCB_DONTCACHE_DEFER avoids per-segment writeback kicks.  However,
generic_write_sync() is not updated for IOCB_DONTCACHE_DEFER:

include/linux/fs.h:generic_write_sync() {
	if (iocb_is_dsync(iocb)) {
		...
	} else if (iocb->ki_flags & IOCB_DONTCACHE) {
		filemap_dontcache_kick_writeback(iocb->ki_filp->f_mapping);
	}
}

During vfs_iocb_iter_write(), generic_write_sync() runs before
filemap_dontcache_edge_complete() is called, while FGP_DONTCACHE_DEFER has
kept the edge folio unmarked.  Then, when filemap_dontcache_edge_complete()
later marks a dirty folio via folio_mark_dropbehind(), it increments
WB_DONTCACHE_DIRTY and filemap_end_dropbehind() returns early because
folio_test_dirty(folio) is true, without calling
filemap_dontcache_kick_writeback(mapping).  If WB_start_dontcache was
already consumed while WB_DONTCACHE_DIRTY was 0, what wakes writeback after
filemap_dontcache_edge_complete() marks the dirty folio?  And shouldn't
generic_write_sync() skip filemap_dontcache_kick_writeback() when
IOCB_DONTCACHE_DEFER is set?

2. If writeback on the shared folio has already started before
filemap_dontcache_edge_complete() locks it, PG_dropbehind was not set when
iomap_add_to_ioend() (or submit_bh_wbc()) submitted the bio:

fs/iomap/ioend.c:iomap_add_to_ioend() {
	if (folio_test_dropbehind(folio))
		bio_set_flag(&ioend->io_bio, BIO_COMPLETE_IN_TASK);
}

Because filemap_dontcache_edge_complete() does not wait for PG_writeback,
it sets PG_dropbehind while the bio is in flight and filemap_end_dropbehind()
returns early because folio_test_writeback(folio) is true.  When that bio
completes in IRQ or softirq context (!in_task()), folio_end_dropbehind()
cannot invalidate the folio; or if the bio completes in task context while
filemap_dontcache_edge_complete() still holds the folio lock,
folio_end_dropbehind()'s folio_trylock() fails.  Won't the folio be left in
the page cache until reclaim in both cases?

[Severity: Medium]
Can calling folio_mark_dropbehind(folio) followed immediately by
filemap_end_dropbehind(folio) in filemap_dontcache_edge_complete() leak
WB_DONTCACHE_DIRTY?

In filemap_dontcache_edge_complete(), folio_mark_dropbehind() is called
unconditionally even when the folio is already clean after the caller's
sync, setting PG_dropbehind under mapping->i_pages.  Immediately after,
filemap_end_dropbehind() checks folio_test_dirty() and clears PG_dropbehind
without holding mapping->i_pages or adjusting WB_DONTCACHE_DIRTY:

mm/filemap.c:filemap_end_dropbehind() {
	if (folio_test_writeback(folio) || folio_test_dirty(folio))
		return;
	if (!folio_test_clear_dropbehind(folio))
		return;
	if (mapping && !folio_mapped(folio))
		folio_unmap_invalidate(mapping, folio, 0);
}

If a concurrent path that dirties a mapped folio without holding the folio
lock (such as zap_present_folio_ptes(), try_to_unmap_one(), or
follow_page_pte() calling folio_mark_dirty() -> filemap_dirty_folio() under
the PTE lock) runs after filemap_end_dropbehind() sees
!folio_test_dirty(folio) but before folio_test_clear_dropbehind(folio):

1. folio_account_dirtied() runs under mapping->i_pages xa_lock, sees
folio_test_dropbehind(folio) is true, and increments WB_DONTCACHE_DIRTY.

2. filemap_end_dropbehind() clears PG_dropbehind via
folio_test_clear_dropbehind(folio) without decrementing WB_DONTCACHE_DIRTY
(unlike filemap_clear_dropbehind()).

3. The folio is not invalidated (either because folio_mapped(folio) is true
or folio_unmap_invalidate() sees folio_test_dirty(folio) under xa_lock), and
when the folio is later cleaned or truncated, PG_dropbehind is already clear
so WB_DONTCACHE_DIRTY is never decremented.

A similar mismatch occurs if PAGECACHE_TAG_DIRTY is still set in
mapping->i_pages while PG_dirty and PG_writeback are clear (for instance,
after folio_clear_dirty_for_io() when writeback bails out early past EOF, or
after folio_cancel_dirty()): folio_mark_dropbehind() sees PAGECACHE_TAG_DIRTY
and increments WB_DONTCACHE_DIRTY, and then filemap_end_dropbehind() sees
!folio_test_writeback(folio) && !folio_test_dirty(folio) and clears
PG_dropbehind without decrementing WB_DONTCACHE_DIRTY.

Should filemap_dontcache_edge_complete() avoid setting PG_dropbehind on an
already-clean folio (or use filemap_clear_dropbehind() under xa_lock when
clearing it)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023024548.71123-1-snitzer@kernel.org?part=6

Re: [PATCH v8 7/7] NFSD: drop a shared boundary page only once both WRITEs have written it

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nfsd: nfsd_write_dio_seg_hold() uses PAGE_SIZE instead of min folio size
--

commit 49f26acfa03e4ed2bb366e82b2cb9296ee952e69
Author: Mike Snitzer <snitzer@kernel.org>

NFSD: drop a shared boundary page only once both WRITEs have written it

This commit issues buffered DONTCACHE segments of direct-mode WRITEs with
IOCB_DONTCACHE_DEFER so that NFSD can defer marking shared boundary pages
drop-behind until the entire WRITE and its fsync complete. Before writing a
buffered segment, NFSD checks filemap_dontcache_edge_completes() for partly
covered boundary pages and calls filemap_dontcache_edge_complete() once the
WRITE finishes.

> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index ade00dabef187..f151289345c92 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
[ ... ]
> @@ -1406,6 +1407,65 @@ nfsd_write_dio_seg_init(struct nfsd_write_dio_seg *segment,
[ ... ]
> +static void
> +nfsd_write_dio_seg_hold(struct file *file, const struct nfsd_write_dio_seg *seg,
> +			loff_t pos, size_t len, struct nfsd_write_dio_hold *hold)
> +{
> +	struct address_space *mapping = file->f_mapping;
> +
> +	hold->first = pos;
> +	hold->last = pos + len - 1;
> +	hold->complete_first = false;
> +	hold->complete_last = false;
> +	hold->written = 0;
> +	if (seg->claim && len) {
> +		bool one_page = (hold->first >> PAGE_SHIFT) ==
> +				(hold->last >> PAGE_SHIFT);
> +		bool head = hold->first & ~PAGE_MASK;
> +		bool tail = (hold->last + 1) & ~PAGE_MASK;
> +
> +		/* Only a page the segment covers partly is shared. */
> +		if (head || (one_page && tail))
> +			hold->complete_first =
> +				filemap_dontcache_edge_completes(mapping, hold->first);
> +		if (!one_page && tail)
> +			hold->complete_last =
> +				filemap_dontcache_edge_completes(mapping, hold->last);
> +	}
> +}

[Severity: Medium]
Should nfsd_write_dio_seg_hold() compute one_page, head, and tail using the
mapping's minimum folio size rather than PAGE_SHIFT and PAGE_MASK?

In nfsd_write_dio_iters_init(), misaligned direct WRITEs are split at
mapping_min_folio_nrbytes(nf->nf_file->f_mapping) boundaries:

	cut = max3(offset_align, PAGE_SIZE,
		   mapping_min_folio_nrbytes(nf->nf_file->f_mapping));

And during vfs_iocb_iter_write(), iomap_get_folio() treats a folio as a
partly covered edge unit whenever the write does not cover a full
mapping_min_folio_nrbytes() unit, setting FGP_DONTCACHE_EDGE and
FGP_DONTCACHE_DEFER so the page cache leaves the folio unmarked for the
caller to complete:

	if (iter->flags & IOMAP_DONTCACHE) {
		size_t unit = mapping_min_folio_nrbytes(iter->inode->i_mapping);
		size_t off = pos & (unit - 1);
		...
		if (off || len < unit) {
			fgp |= FGP_DONTCACHE_EDGE;
			if (iter->flags & IOMAP_DONTCACHE_DEFER)
				fgp |= FGP_DONTCACHE_DEFER;
			len = min_t(size_t, len, unit - off);
		}
	}

Likewise, filemap_dontcache_edge_completes() and
filemap_dontcache_edge_complete() check filemap_dontcache_edge_unit(), which
tests folio_nr_pages(folio) == mapping_min_folio_nrpages(mapping).

On a large-block-size filesystem where mapping_min_folio_nrbytes(mapping) is
larger than PAGE_SIZE (for example, 16 KiB blocks with 4 KiB pages), if two
neighbouring WRITEs split a 16 KiB minimum-order folio on a 4 KiB PAGE_SIZE
boundary, iomap_get_folio() defers dropbehind marking on that folio via
FGP_DONTCACHE_DEFER, but head and tail in nfsd_write_dio_seg_hold() both
evaluate to false. Wouldn't that leave the shared folio unmarked in the page
cache until reclaim, or cause a partial segment spanning multiple base pages
within the same minimum-order folio to treat hold->first and hold->last as
two separate pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023024548.71123-1-snitzer@kernel.org?part=7