On Mon, Jul 01, 2024 at 08:01:30AM +1000, NeilBrown wrote: > On Sun, 30 Jun 2024, Chuck Lever wrote: > > On Fri, Jun 28, 2024 at 05:10:53PM -0400, Mike Snitzer wrote: > > > This is nfs-localio code which blurs the boundary between server and > > > client... > > > > > > The change_attr is used by NFS to detect if a file might have changed. > > > This code is used to get the attributes after a write request. NFS > > > uses a GETATTR request to the server at other times. The change_attr > > > should be consistent between the two else comparisons will be > > > meaningless. > > > > > > So nfs_localio_vfs_getattr() should use the same change_attr as the > > > one that would be used if the NFS GETATTR request were made. For > > > NFSv3, that is nfs_timespec_to_change_attr() as was already > > > implemented. For NFSv4 it is something different (as implemented in > > > this commit). > > > > > > [above header derived from linux-nfs message Neil sent on this topic] > > > > Instead of this note, I recommend: > > > > Message-Id: <171918165963.14261.959545364150864599@xxxxxxxxxxxxxxxxxxxxx> > > Linus would not be impressed. He likes links that you can click on and > follow. I've read email that suggests he doesn't like those either. Another day ending in "y", I guess. > So > Link: https://lore.kernel.org/171918165963.14261.959545364150864599@xxxxxxxxxxxxxxxxxxxxx > > is preferred (at least I think that is the current state of the > conversation > > see https://lore.kernel.org/all/CAHk-=wiD9du3fBHuLYzwUSdNgY+hxMZEWNZpqJXy-=wD2wafdg@xxxxxxxxxxxxxx/ As I read it, this refers to using Message-Id: to link to a patch submission. I'm linking to a discussion thread, not to a patch. Just to be clear. > NeilBrown > > > > > > > > Suggested-by: NeilBrown <neil@xxxxxxxxxx> > > > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > > --- > > > fs/nfs/localio.c | 48 +++++++++++++++++++++++++++++++++++++++--------- > > > 1 file changed, 39 insertions(+), 9 deletions(-) > > > > > > diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c > > > index 0f7d6d55087b..fe96f05ba8ca 100644 > > > --- a/fs/nfs/localio.c > > > +++ b/fs/nfs/localio.c > > > @@ -364,21 +364,47 @@ nfs_set_local_verifier(struct inode *inode, > > > verf->committed = how; > > > } > > > > > > +/* Factored out from fs/nfsd/vfs.h:fh_getattr() */ > > > +static int __vfs_getattr(struct path *p, struct kstat *stat, int version) > > > +{ > > > + u32 request_mask = STATX_BASIC_STATS; > > > + > > > + if (version == 4) > > > + request_mask |= (STATX_BTIME | STATX_CHANGE_COOKIE); > > > + return vfs_getattr(p, stat, request_mask, AT_STATX_SYNC_AS_STAT); > > > +} > > > + > > > +/* > > > + * Copied from fs/nfsd/nfsfh.c:nfsd4_change_attribute(), > > > + * FIXME: factor out to common code. > > > + */ > > > +static u64 __nfsd4_change_attribute(const struct kstat *stat, > > > + const struct inode *inode) > > > +{ > > > + u64 chattr; > > > + > > > + if (stat->result_mask & STATX_CHANGE_COOKIE) { > > > + chattr = stat->change_cookie; > > > + if (S_ISREG(inode->i_mode) && > > > + !(stat->attributes & STATX_ATTR_CHANGE_MONOTONIC)) { > > > + chattr += (u64)stat->ctime.tv_sec << 30; > > > + chattr += stat->ctime.tv_nsec; > > > + } > > > + } else { > > > + chattr = time_to_chattr(&stat->ctime); > > > + } > > > + return chattr; > > > +} > > > + > > > static void nfs_local_vfs_getattr(struct nfs_local_kiocb *iocb) > > > { > > > struct kstat stat; > > > struct file *filp = iocb->kiocb.ki_filp; > > > struct nfs_pgio_header *hdr = iocb->hdr; > > > struct nfs_fattr *fattr = hdr->res.fattr; > > > + int version = NFS_PROTO(hdr->inode)->version; > > > > > > - if (unlikely(!fattr) || vfs_getattr(&filp->f_path, &stat, > > > - STATX_INO | > > > - STATX_ATIME | > > > - STATX_MTIME | > > > - STATX_CTIME | > > > - STATX_SIZE | > > > - STATX_BLOCKS, > > > - AT_STATX_SYNC_AS_STAT)) > > > + if (unlikely(!fattr) || __vfs_getattr(&filp->f_path, &stat, version)) > > > return; > > > > > > fattr->valid = (NFS_ATTR_FATTR_FILEID | > > > @@ -394,7 +420,11 @@ static void nfs_local_vfs_getattr(struct nfs_local_kiocb *iocb) > > > fattr->atime = stat.atime; > > > fattr->mtime = stat.mtime; > > > fattr->ctime = stat.ctime; > > > - fattr->change_attr = nfs_timespec_to_change_attr(&fattr->ctime); > > > + if (version == 4) { > > > + fattr->change_attr = > > > + __nfsd4_change_attribute(&stat, file_inode(filp)); > > > + } else > > > + fattr->change_attr = nfs_timespec_to_change_attr(&fattr->ctime); > > > fattr->du.nfs3.used = stat.blocks << 9; > > > } > > > > > > -- > > > 2.44.0 > > > > > > > -- > > Chuck Lever > > > -- Chuck Lever
