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.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
