Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 30 Nov 2020 21:28:34 -0500
From:   "J. Bruce Fields" <bfields@redhat.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     "bfields@fieldses.org" <bfields@fieldses.org>,
        "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
        "chuck.lever@oracle.com" <chuck.lever@oracle.com>
Subject: Re: [PATCH 1/6] nfsd: add a new EXPORT_OP_NOWCC flag to struct
 export_operations
Message-ID: <20201201022834.GA241188@pick.fieldses.org>
References: <20201130212455.254469-1-trondmy@kernel.org>
 <20201130212455.254469-2-trondmy@kernel.org>
 <20201130225842.GA22446@fieldses.org>
 <1b525278a9a7541529290588a83852a0754cee3e.camel@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1b525278a9a7541529290588a83852a0754cee3e.camel@hammerspace.com>
Precedence: bulk

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@kernel.org?wrote:
> > > From: Jeff Layton <jeff.layton@primarydata.com>
> > > 
> > > 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....

--b.

> 
> That is why this patch is labelled as being an optimisation for NFSv3
> only in the comments above.
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
>