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?
--b.
