Re: [PATCH 1/2] NFSv4/pNFS: prevent i_size regression while layoutcommit is outstanding

[Lucheng Bao]

On Thu, Oct 8, 2026 at 9:43 PM Trond Myklebust <trondmy@xxxxxxxxxx> wrote:
>
> On Thu, 2026-10-08 at 22:28 +0000, Lucheng Bao wrote:
> > Commit ac46bd374c9a ("pNFS: Ensure we layoutcommit before
> > revalidating
> > attributes") replaced outstanding-layoutcommit attribute filtering
> > with
> > synchronization before explicit revalidation. For layouts requiring
> > LAYOUTCOMMIT, OPEN attributes can still shrink i_size before metadata
> > synchronization completes.
>
> The client can't police OPEN. It has no idea which file will be
> affected (particularly when doing an open-by-filename) so unlike the
> case of an explicit truncate() call, it can't serialise with the
> truncate.
> It is therefore up to the server to resolve any ambiguity, either by
> recalling or revoking the layout before allowing the OPEN truncate to
> proceed.
>

Thanks for the quick review. I think the OPEN-truncate is a different
scenario from the one this patch is targeting, and my changelog invited
that reading -- let me describe the actual race.

Agreed for a truncating OPEN (or a SETATTR size), that's a real size
change, the client can't serialise an open-by-name against it, and the
server has to recall or revoke conflicting layouts. This patch isn't
about that case and doesn't touch that path.

The race here involves no truncate and no second client. It's a single
client doing O_APPEND over an nconnect mount:

  1. the client WRITEs the appended data to the data server and it
     completes;
  2. the client arms LAYOUTCOMMIT -- the MDS does not know the new
     size yet;
  3. before that LAYOUTCOMMIT is processed, the client's own next OPEN
     carries a GETATTR, and the MDS answers it with the pre-commit size;
  4. the OPEN reply and the LAYOUTCOMMIT reply travel on different
     nconnect connections and are processed out of order, so the
     smaller, stale size is applied to i_size after the writes;
  5. the next O_APPEND then computes its offset from the regressed
     i_size and overwrites the previous append.

So the smaller size isn't a truncate, it's the MDS size legitimately
lagging until LAYOUTCOMMIT (RFC 8881 section 12.5.4 "It is important to
note that the level of synchronization is from the point of view of the
client that sent the LAYOUTCOMMIT. For file-based layouts, synchronization
of attributes between the metadata and storage devices, primarily the size
attribute, is required."). The check is in the generic size path and keys only
on whether this client has an outstanding LAYOUTCOMMIT for data it has
written but not yet committed.

This is the same protection the client already applies to buffered
writes via !nfs_have_writebacks(): don't let the server shrink i_size
below data the client has written but the server doesn't know about yet.
The only difference is the "server doesn't know yet" window. For plain
buffered I/O it's "dirty pages not yet sent"; for a layout requiring
LAYOUTCOMMIT it's "written to the data server, LAYOUTCOMMIT
outstanding." In fact, once the DS write completes, the pages are no
longer counted by nfs_have_writebacks(), yet the size still isn't known
to the MDS until the commit, that gap is exactly what this extends the
existing check to cover.

> >
> > Commit d8c951c313ed ("NFSv4.1: Don't trust attributes if a pNFS
> > LAYOUTCOMMIT is outstanding") moved LAYOUTCOMMIT post-op attribute
> > processing after cleanup, leaving another unprotected window after
> > NFS_INO_LAYOUTCOMMITTING is cleared.
>
> This is unrelated to the above problem, so fixes for this issue need to
> be in a separate patch.
>

Will fix in v2.

> >
> > Extend the existing writeback checks to reject smaller sizes while a
> > LAYOUTCOMMIT is pending or in flight, preserving size increases and
> > WCC
> > checks. Process the reply's attributes and establish their generation
> > barrier before cleanup removes that protection.
> >
> > Fixes: d8c951c313ed ("NFSv4.1: Don't trust attributes if a pNFS
> > LAYOUTCOMMIT is outstanding")
> > Fixes: ac46bd374c9a ("pNFS: Ensure we layoutcommit before
> > revalidating attributes")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Lucheng Bao <lubao@xxxxxxxxxxxxxxxx>
> > ---
> >  fs/nfs/inode.c    | 8 ++++++--
> >  fs/nfs/nfs4proc.c | 2 +-
> >  2 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 3022454f7698..11432f521b04 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -1636,7 +1636,9 @@ static void nfs_wcc_update_inode(struct inode
> > *inode, struct nfs_fattr *fattr)
> >       if ((fattr->valid & NFS_ATTR_FATTR_PRESIZE)
> >                       && (fattr->valid & NFS_ATTR_FATTR_SIZE)
> >                       && i_size_read(inode) ==
> > nfs_size_to_loff_t(fattr->pre_size)
> > -                     && !nfs_have_writebacks(inode)) {
> > +                     && !nfs_have_writebacks(inode)
> > +                     && (nfs_size_to_loff_t(fattr->size) >=
> > i_size_read(inode)
> > +                             ||
> > !pnfs_layoutcommit_outstanding(inode))) {
>
> Under what circumstance is the client legally supposed to be able to
> call nfs_wcc_update_inode() with NFS_ATTR_FATTR_PRESIZE set, while
> there is an outstanding layoutcommit?
>

It is nfs_post_op_update_inode_force_wcc() from
nfs4_layoutcommit_release(). With
nconnect and overlapping LAYOUTCOMMITS, an earlier LAYOUTCOMMIT can complete
and apply its (smaller) committed size via force_wcc while a later
LAYOUTCOMMIT --
for data already written past that point -- is still in flight.