Linux filesystem development
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: Chuck Lever <cel@kernel.org>
Cc: Jeff Layton <jlayton@kernel.org>,
	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 (Oracle)" <willy@infradead.org>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v8 1/7] NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio
Date: Fri, 23 Oct 2026 21:11:58 -0400	[thread overview]
Message-ID: <atwF3s3-VtUkooZI@kernel.org> (raw)
In-Reply-To: <36498551-15ee-4a55-9757-54aa8a07085a@app.fastmail.com>

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

  reply	other threads:[~2026-10-24  1:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23  2:45 [PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 1/7] NFSD: cut a misaligned direct-mode WRITE at the mapping's smallest folio Mike Snitzer
2026-10-23 17:45   ` Chuck Lever
2026-10-24  1:11     ` Mike Snitzer [this message]
2026-10-23  2:45 ` [PATCH v8 2/7] mm: take a folio off WB_DONTCACHE_DIRTY only if it was counted there Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 3/7] mm: expose WB_DONTCACHE_DIRTY in bdi debugfs stats Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 4/7] mm: add folio_mark_dropbehind() to mark a folio dropbehind and account it Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 5/7] mm, iomap: keep a partly covered page until an uncached write completes it Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 6/7] mm, iomap: let an uncached writer complete its shared pages itself Mike Snitzer
2026-10-23  2:45 ` [PATCH v8 7/7] NFSD: drop a shared boundary page only once both WRITEs have written it Mike Snitzer
2026-10-23 10:57 ` [PATCH v8 0/7] mm, iomap, NFSD: stop uncached writes reading back shared pages Jeff Layton
2026-10-23 14:12   ` 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=atwF3s3-VtUkooZI@kernel.org \
    --to=snitzer@kernel.org \
    --cc=Dai.Ngo@oracle.com \
    --cc=axboe@kernel.dk \
    --cc=cel@kernel.org \
    --cc=corbet@lwn.net \
    --cc=hch@lst.de \
    --cc=jlayton@kernel.org \
    --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=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