Trond, Thanks for reviewing the change. I understand that, logically, if a WRITE is FILE_SYNC, a LAYOUTCOMMIT isn’t necessary. I’m working on enabling file_layout on some legacy POSIX filesystems, where synchronizing metadata between the MDS and DS during client writes to the DS incurs significant overhead. To address this, I’m suggesting an option that allows the MDS to update the metadata. From my reading, RFC8881 doesn’t seem to prohibit LAYOUTCOMMIT in this scenario, could you share more detail about how it would break the FILE_SYNC case? Thanks Haihua On Tue, Sep 2, 2025 at 2:18 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Tue, 2025-09-02 at 18:19 +0000, Haihua Yang wrote: > > [You don't often get email from yanghh@xxxxxxxxx. Learn why this is > > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > --- > > fs/nfs/filelayout/filelayout.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/fs/nfs/filelayout/filelayout.c > > b/fs/nfs/filelayout/filelayout.c > > index d39a1f58e18d..1cb8f413a665 100644 > > --- a/fs/nfs/filelayout/filelayout.c > > +++ b/fs/nfs/filelayout/filelayout.c > > @@ -48,6 +48,8 @@ MODULE_LICENSE("GPL"); > > MODULE_AUTHOR("Dean Hildebrand <dhildebz@xxxxxxxxx>"); > > MODULE_DESCRIPTION("The NFSv4 file layout driver"); > > > > +static bool force_layoutcommit = false; > > + > > #define FILELAYOUT_POLL_RETRY_MAX (15*HZ) > > static const struct pnfs_commit_ops filelayout_commit_ops; > > > > @@ -233,10 +235,11 @@ filelayout_set_layoutcommit(struct > > nfs_pgio_header *hdr) > > { > > loff_t end_offs = 0; > > > > - if (FILELAYOUT_LSEG(hdr->lseg)->commit_through_mds || > > - hdr->res.verf->committed == NFS_FILE_SYNC) > > + if (!force_layoutcommit && (FILELAYOUT_LSEG(hdr->lseg)- > > >commit_through_mds || > > + hdr->res.verf->committed == NFS_FILE_SYNC)) > > return; > > - if (hdr->res.verf->committed == NFS_DATA_SYNC) > > + if (hdr->res.verf->committed == NFS_DATA_SYNC || > > + (force_layoutcommit && hdr->res.verf->committed == > > NFS_FILE_SYNC)) > > end_offs = hdr->mds_offset + (loff_t)hdr->res.count; > > > > /* Note: if the write is unstable, don't set end_offs until > > commit */ > > @@ -1149,5 +1152,8 @@ static void __exit nfs4filelayout_exit(void) > > > > MODULE_ALIAS("nfs-layouttype4-1"); > > > > +module_param(force_layoutcommit, bool, 0644); > > +MODULE_PARM_DESC(force_layoutcommit, "Force LAYOUTCOMMIT after data > > was written (default: false)"); > > + > > module_init(nfs4filelayout_init); > > module_exit(nfs4filelayout_exit); > > -- > > 2.51.0.87.g1fa68948c3.dirty > > > > NACK. > > Firstly, there is no documentation that tells either me or any > pnfs/files user when it would be desirable to set this flag. There > isn't even a reference to what problem in RFC8881 it addresses. > > Secondly, it breaks the NFS_FILE_SYNC case. > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trondmy@xxxxxxxxxx, trond.myklebust@xxxxxxxxxxxxxxx
