From: Jeff Layton <jlayton@kernel.org>
To: Mike Snitzer <snitzer@hammerspace.com>, Chuck Lever <cel@kernel.org>
Cc: linux-nfs@vger.kernel.org, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
Matthew Wilcox <willy@infradead.org>,
Jonathan Corbet <corbet@lwn.net>,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v7 4/5] mm, iomap: let an uncached writer complete its shared pages itself
Date: Wed, 21 Oct 2026 06:54:14 -0400 [thread overview]
Message-ID: <39b14c2a978ec71138492debe95d883aebc3cfe5.camel@kernel.org> (raw)
In-Reply-To: <20261020220346.28020-5-snitzer@kernel.org>
On Tue, 2026-10-20 at 18:03 -0400, Mike Snitzer wrote:
> 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.
>
> 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.
>
> Assisted-by: LLM
> 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 | 58 ++++++++++++++++++++++++++++++++++++++++-
> 6 files changed, 77 insertions(+), 3 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 780a0186045d..5ca82000e2ec 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -811,6 +811,8 @@ struct folio *iomap_get_folio(struct iomap_iter *iter, loff_t pos, size_t len)
> */
> if (offset_in_page(pos) || len < PAGE_SIZE) {
> fgp |= FGP_DONTCACHE_EDGE;
> + if (iter->flags & IOMAP_DONTCACHE_DEFER)
> + fgp |= FGP_DONTCACHE_DEFER;
> len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
> }
> }
> @@ -1318,6 +1320,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 0397ec771547..7d7c56a42643 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1675,6 +1675,61 @@ void folio_mark_dropbehind(struct folio *folio)
> }
> EXPORT_SYMBOL_GPL(folio_mark_dropbehind);
>
> +/**
> + * 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);
> + 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.
> + */
> +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;
> + folio_mark_dropbehind(folio);
> + folio_unlock(folio);
> + folio_end_dropbehind(folio);
> + folio_put(folio);
> +}
> +EXPORT_SYMBOL_GPL(filemap_dontcache_edge_complete);
> +
> /**
> * folio_end_writeback_no_dropbehind - End writeback against a folio.
> * @folio: The folio.
> @@ -2024,7 +2079,8 @@ struct folio *__filemap_get_folio_mpol(struct address_space *mapping,
> * folio lock, so the writeback that cleans both writes' data drops it.
> */
> if ((fgp_flags & FGP_DONTCACHE) && (fgp_flags & FGP_DONTCACHE_EDGE) &&
> - (fgp_flags & FGP_LOCK) && folio_test_uptodate(folio))
> + !(fgp_flags & FGP_DONTCACHE_DEFER) && (fgp_flags & FGP_LOCK) &&
> + folio_test_uptodate(folio))
> folio_mark_dropbehind(folio);
> no_page:
> if (!folio && (fgp_flags & FGP_CREAT)) {
2 comments from Chuck's sashiko on this one:
[Severity: Low]
Could filemap_dontcache_edge_complete() in mm/filemap.c avoid missing the
immediate drop when another task acquires the folio lock between
folio_unlock() and folio_end_dropbehind()? In that case folio_end_dropbehind()
fails its folio_trylock() and does not retry; if the folio is already clean,
there may be no later writeback completion to drop it, so it can remain cached
until unrelated activity or reclaim.
[Severity: Medium]
Could filemap_dontcache_edge_complete() in mm/filemap.c restrict marking to
the boundary page when the mapping returns a large folio? iomap_get_folio()
in fs/iomap/buffered-io.c limits a partial edge request to one page, but the
mapping's minimum folio order can still cause __filemap_get_folio_mpol() to
return a multi-page folio, or the lookup can find an existing large folio.
This helper then marks the entire folio for drop-behind. For example, if a
caller uses deferred completion on an XFS mapping with a minimum folio order,
marking one edge can account the whole dirty folio for DONTCACHE writeback or
invalidate the whole clean, unmapped folio. Could this path handle only the
page covered by the completed edge?
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2026-10-21 10:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-20 22:03 [PATCH v7 0/5] mm, iomap, NFSD: stop uncached writes reading back shared pages Mike Snitzer
2026-10-20 22:03 ` [PATCH v7 1/5] mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats Mike Snitzer
2026-10-20 22:11 ` Mike Snitzer
2026-10-21 10:50 ` Jeff Layton
2026-10-20 22:03 ` [PATCH v7 2/5] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it Mike Snitzer
2026-10-21 10:52 ` Jeff Layton
2026-10-20 22:03 ` [PATCH v7 3/5] mm, iomap: keep a partly covered page until an uncached write completes it Mike Snitzer
2026-10-20 22:03 ` [PATCH v7 4/5] mm, iomap: let an uncached writer complete its shared pages itself Mike Snitzer
2026-10-21 10:54 ` Jeff Layton [this message]
2026-10-20 22:03 ` [PATCH v7 5/5] NFSD: drop a shared boundary page only once both WRITEs have written it Mike Snitzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=39b14c2a978ec71138492debe95d883aebc3cfe5.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=axboe@kernel.dk \
--cc=cel@kernel.org \
--cc=corbet@lwn.net \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=snitzer@hammerspace.com \
--cc=tom@talpey.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.