Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Mike Snitzer <snitzer@kernel.org>, 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>,
	 linux-mm@kvack.org, Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH v4 03/10] NFSD: only split a direct-mode WRITE for a worthwhile direct middle
Date: Fri, 16 Oct 2026 07:03:13 -0400	[thread overview]
Message-ID: <13ffd6a9929976935ed2052fecc85a7345a07c36.camel@kernel.org> (raw)
In-Reply-To: <20261015233309.42999-4-snitzer@kernel.org>

On Thu, 2026-10-15 at 19:33 -0400, Mike Snitzer wrote:
> Splitting a misaligned direct-mode WRITE issues it as two or three
> writes instead of one, and the direct write first writes back and
> invalidates the page cache over its range.  When the DIO-aligned middle
> is small, the WRITE is mostly its buffered prefix and suffix anyway, and
> the direct middle only adds that overhead.
> 
> Add direct_misaligned_min_bytes (debugfs, default 8192) and do not split
> a WRITE that has a misaligned start or end and a middle smaller than that
> many bytes.  Such a WRITE is issued as a single buffered segment, like
> the other WRITEs NFSD does not split.
> 
> The gate counts bytes, not pages.  What it weighs, the cost of three I/Os
> against one, does not change with the page size, and since a misaligned
> WRITE's middle is cut at page boundaries, the middle of such a WRITE is
> whole pages: a gate of two pages would demand a 32 KiB middle on 16 KiB
> pages and 128 KiB on 64 KiB pages before splitting at all.  On 4 KiB
> pages the default is two pages.
> 
> Also decide each segment's IOCB_* flags in nfsd_write_dio_iters_init()
> instead of in the write loop of nfsd_direct_write().  The flags do not
> change: the loop already set IOCB_DONTCACHE on every segment without
> IOCB_DIRECT when the file system supports FOP_DONTCACHE.
> 
> The threshold, counted there in pages, and the move of the flag
> decisions come from Christoph's suggested diff [1].  Two of its choices
> are not taken.  It issued the prefix, the suffix and any WRITE it did
> not split, whether smaller than its alignment or with a middle below the
> threshold, as cached buffered I/O, to leave partial pages in place for
> read-modify-write; this patch keeps the DONTCACHE policy already in the
> tree, and a later commit makes that policy selectable.  It also split a
> WRITE whose payload is misaligned in memory and issued the middle as
> DONTCACHE buffered I/O, so that a large middle stayed out of the page
> cache while the cached ends stayed in it.  Here the prefix and suffix
> are DONTCACHE as well, so that split would issue the same uncached
> buffered I/O as two or three writes instead of one; such a WRITE is
> issued whole, like every other WRITE NFSD does not split.
> 
> Document in nfsd-io-modes.rst when NFSD does not split a WRITE.
> 
> Link: https://lore.kernel.org/linux-nfs/aRShjU_Ti7f2Ci7I@infradead.org/ [1]
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Assisted-by: LLM
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  .../filesystems/nfs/nfsd-io-modes.rst         | 13 ++++++
>  fs/nfsd/debugfs.c                             | 12 +++++
>  fs/nfsd/nfsd.h                                |  1 +
>  fs/nfsd/vfs.c                                 | 46 +++++++++++--------
>  4 files changed, 53 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/filesystems/nfs/nfsd-io-modes.rst b/Documentation/filesystems/nfs/nfsd-io-modes.rst
> index f9b63f1a33990..eab35a9bbdb93 100644
> --- a/Documentation/filesystems/nfs/nfsd-io-modes.rst
> +++ b/Documentation/filesystems/nfs/nfsd-io-modes.rst
> @@ -137,6 +137,19 @@ Misaligned WRITE:
>      cannot invalidate page cache that overlaps the segment. The
>      iomap_dio_invalidate_fail trace event reports such a fallback.
>  
> +    If NFSD does not split a misaligned WRITE, it issues the whole WRITE
> +    as a single DONTCACHE buffered IO (normal buffered IO if the
> +    filesystem lacks FOP_DONTCACHE). NFSD does not split a WRITE when:
> +
> +    - the filesystem advertises no DIO alignment;
> +    - the WRITE is smaller than the larger of the offset and memory
> +      alignments;
> +    - the WRITE has a misaligned start or end and its DIO-aligned middle
> +      is smaller than /sys/kernel/debug/nfsd/direct_misaligned_min_bytes
> +      bytes (default 8192);
> +    - the WRITE payload is not aligned in memory to the block device's
> +      dma_alignment, so the middle cannot be O_DIRECT either.
> +
>  Tracing:
>      The nfsd_read_direct trace event shows how NFSD expands any
>      misaligned READ to the next DIO-aligned block (on either end of the
> diff --git a/fs/nfsd/debugfs.c b/fs/nfsd/debugfs.c
> index 386fd1c54f527..02b0369f17648 100644
> --- a/fs/nfsd/debugfs.c
> +++ b/fs/nfsd/debugfs.c
> @@ -140,6 +140,18 @@ void nfsd_debugfs_init(void)
>  
>  	debugfs_create_file("io_cache_write", 0644, nfsd_top_dir, NULL,
>  			    &nfsd_io_cache_write_fops);
> +
> +	/*
> +	 * /sys/kernel/debug/nfsd/direct_misaligned_min_bytes
> +	 *
> +	 * Minimum size, in bytes, of the DIO-aligned middle segment for NFSD
> +	 * to split a misaligned direct-mode WRITE.  A WRITE with a misaligned
> +	 * start or end and a smaller middle is issued as a single buffered
> +	 * segment.  It is in bytes so that it means the same on every page
> +	 * size.  The default value of this setting is 8192.
> +	 */
> +	debugfs_create_u32("direct_misaligned_min_bytes", 0644, nfsd_top_dir,
> +			   &nfsd_direct_misaligned_min_bytes);


This is fine for now, but (FYI) I'm working on a patch series to add
r/wiomode= export options, so we'll be able to set the iomode on a per-
export basis in the future.

At that point, we may want to move this and the io_cache_*= settings to
netlink and nfsdctl, so admins will be able to define it in
/etc/nfs.conf. That may also be an argument for making this a per-net
setting instead of global

>  #ifdef CONFIG_NFSD_V4
>  	debugfs_create_bool("delegated_timestamps", 0644, nfsd_top_dir,
>  			    &nfsd_delegts_enabled);
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index d824d2c390294..db81620bd5168 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -142,6 +142,7 @@ enum {
>  
>  extern u64 nfsd_io_cache_read __read_mostly;
>  extern u64 nfsd_io_cache_write __read_mostly;
> +extern u32 nfsd_direct_misaligned_min_bytes __read_mostly;
>  
>  bool nfsd_v4client(struct svc_rqst *rqstp);
>  
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index bd3dab36e2962..a419eee401d9d 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -54,6 +54,7 @@
>  bool nfsd_disable_splice_read __read_mostly;
>  u64 nfsd_io_cache_read __read_mostly = NFSD_IO_BUFFERED;
>  u64 nfsd_io_cache_write __read_mostly = NFSD_IO_BUFFERED;
> +u32 nfsd_direct_misaligned_min_bytes __read_mostly = 8192;
>  
>  /**
>   * nfserrno - Map Linux errnos to NFS errnos
> @@ -1323,9 +1324,13 @@ nfsd_write_dio_iters_init(struct nfsd_file *nf, struct bio_vec *bvec,
>  	u32 mem_align = nf->nf_dio_mem_align;
>  	size_t prefix, middle, suffix;
>  	loff_t offset = iocb->ki_pos;
> +	unsigned int dontcache_flags = 0;
>  	unsigned int nsegs = 0;
>  	u32 cut;
>  
> +	if (nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
> +		dontcache_flags = IOCB_DONTCACHE;
> +
>  	/*
>  	 * Check if direct I/O is feasible for this write request.
>  	 * If alignments are not available, the write is too small,
> @@ -1355,12 +1360,16 @@ nfsd_write_dio_iters_init(struct nfsd_file *nf, struct bio_vec *bvec,
>  	middle = middle_end - prefix_end;
>  	suffix = orig_end - middle_end;
>  
> -	if (!middle)
> +	if (!middle ||
> +	    ((prefix || suffix) &&
> +	     middle < nfsd_direct_misaligned_min_bytes))
>  		goto no_dio;
>  
> -	if (prefix)
> -		nfsd_write_dio_seg_init(&segments[nsegs++], bvec,
> +	if (prefix) {
> +		nfsd_write_dio_seg_init(&segments[nsegs], bvec,
>  					nvecs, total, 0, prefix, iocb);
> +		segments[nsegs++].flags |= dontcache_flags;
> +	}
>  
>  	nfsd_write_dio_seg_init(&segments[nsegs], bvec, nvecs,
>  				total, prefix, middle, iocb);
> @@ -1374,22 +1383,28 @@ nfsd_write_dio_iters_init(struct nfsd_file *nf, struct bio_vec *bvec,
>  	 */
>  	if (iov_iter_bvec_offset(&segments[nsegs].iter) & (mem_align - 1))
>  		goto no_dio;
> -	segments[nsegs].flags |= IOCB_DIRECT;
> -	/* In case the file system falls back to buffered I/O (-ENOTBLK). */
> -	if (nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
> -		segments[nsegs].flags |= IOCB_DONTCACHE;
> -	nsegs++;
> +	/*
> +	 * IOCB_DONTCACHE matters only if the file system falls back to
> +	 * buffered I/O (-ENOTBLK).
> +	 */
> +	segments[nsegs++].flags |= IOCB_DIRECT | dontcache_flags;
>  
> -	if (suffix)
> -		nfsd_write_dio_seg_init(&segments[nsegs++], bvec, nvecs, total,
> +	if (suffix) {
> +		nfsd_write_dio_seg_init(&segments[nsegs], bvec, nvecs, total,
>  					prefix + middle, suffix, iocb);
> +		segments[nsegs++].flags |= dontcache_flags;
> +	}
>  
>  	return nsegs;
>  
>  no_dio:
> -	/* No DIO alignment possible - pack into single non-DIO segment. */
> +	/*
> +	 * Issue the whole WRITE as a single buffered segment, uncached when
> +	 * the file system supports FOP_DONTCACHE.
> +	 */
>  	nfsd_write_dio_seg_init(&segments[0], bvec, nvecs, total, 0,
>  				total, iocb);
> +	segments[0].flags |= dontcache_flags;
>  	return 1;
>  }
>  
> @@ -1413,16 +1428,9 @@ nfsd_direct_write(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  		if (kiocb->ki_flags & IOCB_DIRECT)
>  			trace_nfsd_write_direct(rqstp, fhp, kiocb->ki_pos,
>  						segments[i].iter.count);
> -		else {
> +		else
>  			trace_nfsd_write_vector(rqstp, fhp, kiocb->ki_pos,
>  						segments[i].iter.count);
> -			/*
> -			 * Mark the I/O buffer as evict-able to reduce
> -			 * memory contention.
> -			 */
> -			if (nf->nf_file->f_op->fop_flags & FOP_DONTCACHE)
> -				kiocb->ki_flags |= IOCB_DONTCACHE;
> -		}
>  
>  		expected = iov_iter_count(&segments[i].iter);
>  

-- 
Jeff Layton <jlayton@kernel.org>


  reply	other threads:[~2026-10-16 11:03 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15 23:32 [PATCH v4 00/10] NFSD: keep direct-mode I/O out of the page cache Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 01/10] NFSD: cut a misaligned direct-mode WRITE at page boundaries Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 02/10] NFSD: mark the direct middle of a split WRITE IOCB_DONTCACHE as well Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 03/10] NFSD: only split a direct-mode WRITE for a worthwhile direct middle Mike Snitzer
2026-10-16 11:03   ` Jeff Layton [this message]
2026-10-15 23:33 ` [PATCH v4 04/10] NFSD: add direct_misaligned_dontcache debugfs knob Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 05/10] NFSD: do not use direct I/O for a READ smaller than its alignment Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 06/10] NFSD: persist a synchronous direct-mode WRITE once, after all of its segments Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 07/10] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it Mike Snitzer
2026-10-16 11:08   ` Jeff Layton
2026-10-16 11:54     ` Matthew Wilcox
2026-10-16 18:36       ` Chuck Lever
2026-10-15 23:33 ` [PATCH v4 08/10] mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 09/10] NFSD: keep boundary page of a split direct-mode WRITE until both writers complete Mike Snitzer
2026-10-15 23:33 ` [PATCH v4 10/10] NFSD: add tracing for how direct-mode READ and WRITE are serviced Mike Snitzer
2026-10-16 11:18 ` [PATCH v4 00/10] NFSD: keep direct-mode I/O out of the page cache Jeff Layton
2026-10-16 19:23 ` [PATCH v4 0/10] " Chuck Lever

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=13ffd6a9929976935ed2052fecc85a7345a07c36.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=axboe@kernel.dk \
    --cc=cel@kernel.org \
    --cc=hch@infradead.org \
    --cc=hch@lst.de \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=snitzer@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox