* [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec [not found] <20251105192806.77093-1-cel@kernel.org> @ 2025-11-05 19:28 ` Chuck Lever 2025-11-06 0:55 ` NeilBrown 2025-11-06 13:05 ` Christoph Hellwig 0 siblings, 2 replies; 3+ messages in thread From: Chuck Lever @ 2025-11-05 19:28 UTC (permalink / raw) To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey Cc: linux-nfs, Chuck Lever, Mike Snitzer, stable From: Chuck Lever <chuck.lever@oracle.com> Mike noted that when NFSD responds to an NFS_FILE_SYNC WRITE, it does not also persist file time stamps. To wit, Section 18.32.3 of RFC 8881 mandates: > The client specifies with the stable parameter the method of how > the data is to be processed by the server. If stable is > FILE_SYNC4, the server MUST commit the data written plus all file > system metadata to stable storage before returning results. This > corresponds to the NFSv2 protocol semantics. Any other behavior > constitutes a protocol violation. If stable is DATA_SYNC4, then > the server MUST commit all of the data to stable storage and > enough of the metadata to retrieve the data before returning. Commit 3f3503adb332 ("NFSD: Use vfs_iocb_iter_write()") replaced: - flags |= RWF_SYNC; with: + kiocb.ki_flags |= IOCB_DSYNC; which appears to be correct given: if (flags & RWF_SYNC) kiocb_flags |= IOCB_DSYNC; in kiocb_set_rw_flags(). However the author of that commit did not appreciate that the previous line in kiocb_set_rw_flags() results in IOCB_SYNC also being set: kiocb_flags |= (__force int) (flags & RWF_SUPPORTED); RWF_SUPPORTED contains RWF_SYNC, and RWF_SYNC is the same bit as IOCB_SYNC. Reviewers at the time did not catch the omission. Reported-by: Mike Snitzer <snitzer@kernel.org> Closes: https://lore.kernel.org/linux-nfs/20251018005431.3403-1-cel@kernel.org/T/#t Reviewed-by: Jeff Layton <jlayton@kernel.org> Fixes: 3f3503adb332 ("NFSD: Use vfs_iocb_iter_write()") Cc: stable@vger.kernel.org Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfsd/vfs.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index f537a7b4ee01..5333d49910d9 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1314,8 +1314,18 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, stable = NFS_UNSTABLE; init_sync_kiocb(&kiocb, file); kiocb.ki_pos = offset; - if (stable && !fhp->fh_use_wgather) - kiocb.ki_flags |= IOCB_DSYNC; + if (likely(!fhp->fh_use_wgather)) { + switch (stable) { + case NFS_FILE_SYNC: + /* persist data and timestamps */ + kiocb.ki_flags |= IOCB_DSYNC | IOCB_SYNC; + break; + case NFS_DATA_SYNC: + /* persist data only */ + kiocb.ki_flags |= IOCB_DSYNC; + break; + } + } nvecs = xdr_buf_to_bvec(rqstp->rq_bvec, rqstp->rq_maxpages, payload); iov_iter_bvec(&iter, ITER_SOURCE, rqstp->rq_bvec, nvecs, *cnt); -- 2.51.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec 2025-11-05 19:28 ` [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec Chuck Lever @ 2025-11-06 0:55 ` NeilBrown 2025-11-06 13:05 ` Christoph Hellwig 1 sibling, 0 replies; 3+ messages in thread From: NeilBrown @ 2025-11-06 0:55 UTC (permalink / raw) To: Chuck Lever Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs, Chuck Lever, Mike Snitzer, stable On Thu, 06 Nov 2025, Chuck Lever wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > Mike noted that when NFSD responds to an NFS_FILE_SYNC WRITE, it > does not also persist file time stamps. To wit, Section 18.32.3 > of RFC 8881 mandates: > > > The client specifies with the stable parameter the method of how > > the data is to be processed by the server. If stable is > > FILE_SYNC4, the server MUST commit the data written plus all file > > system metadata to stable storage before returning results. This > > corresponds to the NFSv2 protocol semantics. Any other behavior > > constitutes a protocol violation. If stable is DATA_SYNC4, then > > the server MUST commit all of the data to stable storage and > > enough of the metadata to retrieve the data before returning. > > Commit 3f3503adb332 ("NFSD: Use vfs_iocb_iter_write()") replaced: > > - flags |= RWF_SYNC; > > with: > > + kiocb.ki_flags |= IOCB_DSYNC; > > which appears to be correct given: > > if (flags & RWF_SYNC) > kiocb_flags |= IOCB_DSYNC; > > in kiocb_set_rw_flags(). However the author of that commit did not "the author and reviewers of that commit" Reviewed-by: NeilBrown <neil@brown.name> Thanks, NeilBrown > appreciate that the previous line in kiocb_set_rw_flags() results > in IOCB_SYNC also being set: > > kiocb_flags |= (__force int) (flags & RWF_SUPPORTED); > > RWF_SUPPORTED contains RWF_SYNC, and RWF_SYNC is the same bit as > IOCB_SYNC. Reviewers at the time did not catch the omission. > > Reported-by: Mike Snitzer <snitzer@kernel.org> > Closes: https://lore.kernel.org/linux-nfs/20251018005431.3403-1-cel@kernel.org/T/#t > Reviewed-by: Jeff Layton <jlayton@kernel.org> > Fixes: 3f3503adb332 ("NFSD: Use vfs_iocb_iter_write()") > Cc: stable@vger.kernel.org > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfsd/vfs.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c > index f537a7b4ee01..5333d49910d9 100644 > --- a/fs/nfsd/vfs.c > +++ b/fs/nfsd/vfs.c > @@ -1314,8 +1314,18 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, > stable = NFS_UNSTABLE; > init_sync_kiocb(&kiocb, file); > kiocb.ki_pos = offset; > - if (stable && !fhp->fh_use_wgather) > - kiocb.ki_flags |= IOCB_DSYNC; > + if (likely(!fhp->fh_use_wgather)) { > + switch (stable) { > + case NFS_FILE_SYNC: > + /* persist data and timestamps */ > + kiocb.ki_flags |= IOCB_DSYNC | IOCB_SYNC; > + break; > + case NFS_DATA_SYNC: > + /* persist data only */ > + kiocb.ki_flags |= IOCB_DSYNC; > + break; > + } > + } > > nvecs = xdr_buf_to_bvec(rqstp->rq_bvec, rqstp->rq_maxpages, payload); > iov_iter_bvec(&iter, ITER_SOURCE, rqstp->rq_bvec, nvecs, *cnt); > -- > 2.51.0 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec 2025-11-05 19:28 ` [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec Chuck Lever 2025-11-06 0:55 ` NeilBrown @ 2025-11-06 13:05 ` Christoph Hellwig 1 sibling, 0 replies; 3+ messages in thread From: Christoph Hellwig @ 2025-11-06 13:05 UTC (permalink / raw) To: Chuck Lever Cc: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs, Chuck Lever, Mike Snitzer, stable Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-06 13:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251105192806.77093-1-cel@kernel.org>
2025-11-05 19:28 ` [PATCH v10 2/5] NFSD: Make FILE_SYNC WRITEs comply with spec Chuck Lever
2025-11-06 0:55 ` NeilBrown
2025-11-06 13:05 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox