[Linux-cachefs] Adventures in NFS re-exporting

[Trond Myklebust]

On Thu, 2020-10-01 at 15:26 -0400, bfields at fieldses.org wrote:
> On Thu, Oct 01, 2020 at 07:24:42PM +0000, Trond Myklebust wrote:
> > On Thu, 2020-10-01 at 14:41 -0400, J. Bruce Fields wrote:
> > > On Wed, Sep 30, 2020 at 03:30:22PM -0400, Jeff Layton wrote:
> > > > On Tue, 2020-09-22 at 13:31 +0100, Daire Byrne wrote:
> > > > > This patch helps to avoid this when applied to the re-export
> > > > > server but there may be other places where this happens too.
> > > > > I
> > > > > accept that this patch is probably not the right/general way
> > > > > to
> > > > > do this, but it helps to highlight the issue when re-
> > > > > exporting
> > > > > and it works well for our use case:
> > > > > 
> > > > > --- linux-5.5.0-1.el7.x86_64/fs/nfs/inode.c     2020-01-27
> > > > > 00:23:03.000000000 +0000
> > > > > +++ new/fs/nfs/inode.c  2020-02-13 16:32:09.013055074 +0000
> > > > > @@ -1869,7 +1869,7 @@
> > > > >  
> > > > >         /* More cache consistency checks */
> > > > >         if (fattr->valid & NFS_ATTR_FATTR_CHANGE) {
> > > > > -               if (!inode_eq_iversion_raw(inode, fattr-
> > > > > > change_attr)) {
> > > > > +               if (inode_peek_iversion_raw(inode) < fattr-
> > > > > > change_attr) {
> > > > >                         /* Could it be a race with writeback?
> > > > > */
> > > > >                         if (!(have_writers ||
> > > > > have_delegation)) {
> > > > >                                 invalid |=
> > > > > NFS_INO_INVALID_DATA
> > > > > 
> > > > > With this patch, the re-export server's NFS client attribute
> > > > > cache is maintained and used by all the clients that then
> > > > > mount
> > > > > it. When many hundreds of clients are all doing similar
> > > > > things at
> > > > > the same time, the re-export server's NFS client cache is
> > > > > invaluable in accelerating the lookups (getattrs).
> > > > > 
> > > > > Perhaps a more correct approach would be to detect when it is
> > > > > knfsd that is accessing the client mount and change the cache
> > > > > consistency checks accordingly? 
> > > > 
> > > > Yeah, I don't think you can do this for the reasons Trond
> > > > outlined.
> > > 
> > > I'm not clear whether Trond thought that knfsd's behavior in the
> > > case
> > > it
> > > returns NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR might be good enough
> > > to
> > > allow
> > > this or some other optimization.
> > > 
> > 
> > NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR should normally be good enough
> > to
> > allow the above optimisation, yes. I'm less sure about whether or
> > not
> > we are correct in returning NFS4_CHANGE_TYPE_IS_MONOTONIC_INCR when
> > in
> > fact we are adding the ctime and filesystem-specific change
> > attribute,
> > but we could fix that too.
> 
> Could you explain your concern?
> 

Same as before: that the ctime could cause the value to regress if
someone messes with the system time on the server. Yes, we do add in
the change attribute, but the value of ctime.tv_sec dominates by a
factor 2^30.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust at hammerspace.com