On Mon, 2020-11-30 at 21:28 -0500, J. Bruce Fields wrote: > On Tue, Dec 01, 2020 at 12:45:16AM +0000, Trond Myklebust wrote: > > On Mon, 2020-11-30 at 17:58 -0500, J. Bruce Fields wrote: > > > This is great, thanks: > > > > > > On Mon, Nov 30, 2020 at 04:24:50PM -0500, > > > trondmy@xxxxxxxxxx wrote: > > > > From: Jeff Layton <jeff.layton@xxxxxxxxxxxxxxx> > > > > > > > > With NFSv3 nfsd will always attempt to send along WCC data to > > > > the > > > > client. This generally involves saving off the in-core inode > > > > information > > > > prior to doing the operation on the given filehandle, and then > > > > issuing a > > > > vfs_getattr to it after the op. > > > > > > > > Some filesystems (particularly clustered or networked ones) > > > > have an > > > > expensive ->getattr inode operation. Atomicitiy is also often > > > > difficult > > > > or impossible to guarantee on such filesystems. For those, > > > > we're > > > > best > > > > off not trying to provide WCC information to the client at all, > > > > and > > > > to > > > > simply allow it to poll for that information as needed with a > > > > GETATTR > > > > RPC. > > > > > > > > This patch adds a new flags field to struct export_operations, > > > > and > > > > defines a new EXPORT_OP_NOWCC flag that filesystems can use to > > > > indicate > > > > that nfsd should not attempt to provide WCC info in NFSv3 > > > > replies. > > > > It > > > > also adds a blurb about the new flags field and flag to the > > > > exporting > > > > documentation. > > > > > > In the v4 case I think it should also turn off the "atomic" flag > > > in > > > the > > > change_info4 structure that's returned by some operations. > > > > > > > To answer this comment (which I missed earlier): I don't know that > > we > > can turn off WCC for NFSv4. The GETATTR is a completely separate > > operation, so the server would have to second-guess what the client > > needs it for in order to optimise it away. > > In the v4 case, we're setting the "atomic" field in the change_info4 > struct to true even though the returned changeattrs clearly aren't > atomic with the operation in the re-export case. > > That atomic field is initialized from fh_post_saved, so we just need > to > set it to false in the v4 case as we are in the v3 case already. > > Yes, it's true, that doesn't allow any optimizations because we still > have to get the post-op change attributes. > > But it's a bug we may as well fix while we're here, and it probably > simplifies this patch if anything.... I'd argue that is a completely separate issue. This is an optimisation for NFSv3, whereas what you're talking about is atomicity (whether in general or for NFSv4 only). I'd therefore prefer to make that a completely separate export flag so that it can be treated as separate functionality. A local filesystem might choose to set the 'non-atomic' flag without wanting to turn off NFSv3 WCC attributes. Yes, the latter are assumed to be atomic, but a number of commercial servers do abuse that assumption in practice. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
