[PATCH v4 11/10] NFSD: mark a shared boundary page only once the other WRITE has written it

[Mike Snitzer <snitzer@kernel.org>]

[incremental fix to v4's 9/10 which will be folded into it for v5]

nfsd_write_dio_boundary_claim() treats finding the shared page already in
the page cache (-EEXIST) as "this WRITE is the second of the two", and the
second marks the page drop-behind once its own segment is written.  But a
folio already there says only that the other WRITE has claimed the page,
not that it has written its part.  WRITEs that arrive together -- the RPCs
of one O_DIRECT write larger than the client's wsize, which the client
sends at the same time -- both claim before either writes, so the "second"
can mark the page, and have it dropped, before the other WRITE's data is
in it.  That WRITE then reads the page back from the device.

Treat the page as written by the other WRITE only if its folio is uptodate
when this WRITE finds it: an empty claimed folio becomes uptodate only once
a write fills it.  When it is not, neither WRITE marks the page, and it is
an ordinary cached page.  It is written back like any dirty page, by the
WRITE's fsync, the client's COMMIT or the flusher, but that writeback does
not drop it, since only a drop-behind folio is dropped, and it is not
counted as DONTCACHE dirty, so it does not wake the DONTCACHE writeback
kick.  It stays in the page cache, clean and reclaimable, until reclaim,
truncate or a later direct write over it removes it: one cached page per
concurrent pair instead of a device read.

Measured with 32 interleaved writers of 47008-byte records, NFSD DIRECT,
4 KiB pages, device reads per WRITE before and after this change:

  WRITEs per record      halves arrive     client      before   after
  1 x 47008              -                 FILE_SYNC   0.0102   0.0014
  32768 + 14240          one, then other   FILE_SYNC   0.0021   0.0011
  32768 + 14240          together          FILE_SYNC   0.0401   0.0030
  32768 + 14240          together          UNSTABLE    0.0944   0.0082

In the concurrent cases 90% (FILE_SYNC) and 95% (UNSTABLE) of the pages
read back before the change were the page where a record's two WRITEs
meet.  After it, 14993 (FILE_SYNC) and 22549 (UNSTABLE) of the 90615
boundary pages, 59 and 88 MiB of the 2 GiB written, were still cached at
the end of the run, against 484 and 403 before; with
direct_misaligned_dontcache=N every boundary page stays cached.

Assisted-by: LLM
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfsd/vfs.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index e30c50b9b518..424fa3b05705 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1330,9 +1330,22 @@ nfsd_write_dio_seg_init(struct nfsd_write_dio_seg *segment,
  * DONTCACHE write marks only a folio it allocates, so neither WRITE marks
  * this one and no writeback drops it before the other WRITE has written it.
  *
- * Return: true if a folio was already there.  This WRITE is then the
- * second of the two and calls nfsd_write_dio_boundary_complete() after
- * writing the page.
+ * A folio already there says only that the other WRITE has claimed the page,
+ * not that it has written its part.  Two WRITEs that arrive together -- the
+ * RPCs of one O_DIRECT write larger than the client's wsize -- both claim the
+ * page before either writes, and a page marked now would be dropped before
+ * the other WRITE's data is in it.  This WRITE completes the page only if the
+ * folio is uptodate, which an empty claimed folio becomes only once a write
+ * fills it.  Otherwise neither WRITE marks the page, and it is an ordinary
+ * cached page: written back by the WRITE's fsync, the client's COMMIT or the
+ * flusher like any dirty page, but not dropped by that writeback, which drops
+ * only a drop-behind folio, and not counted as DONTCACHE dirty, so it does
+ * not wake the DONTCACHE writeback kick.  It leaves the page cache when
+ * reclaim, truncate or a later direct write over it removes it.
+ *
+ * Return: true if the other WRITE has already written the page.  This WRITE
+ * is then the second of the two and calls nfsd_write_dio_boundary_complete()
+ * after writing the page.
  */
 static bool
 nfsd_write_dio_boundary_claim(struct file *file, loff_t pos)
@@ -1354,7 +1367,16 @@ nfsd_write_dio_boundary_claim(struct file *file, loff_t pos)
 	if (!err)
 		folio_unlock(folio);
 	folio_put(folio);
-	return err == -EEXIST;
+	if (err != -EEXIST)
+		return false;
+
+	/* 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;
+	err = folio_test_uptodate(folio);
+	folio_put(folio);
+	return err;
 }
 
 /*
-- 
2.52.0