Re: [PATCH v2 4/4] NFSD: handle GETATTR conflict with write delegation

From: Jeff Layton <jlayton@kernel.org>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: dai.ngo@oracle.com, chuck.lever@oracle.com,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] NFSD: handle GETATTR conflict with write delegation
Date: Mon, 15 May 2023 16:21:59 -0400	[thread overview]
Message-ID: <30df13a02cbe9d7c72d0518c011e066e563bcbc8.camel@kernel.org> (raw)
In-Reply-To: <CAN-5tyFE3=DF+48SBGrC2u3y-MNr+1nM+xFM4CXaYv23AMZvew@mail.gmail.com>

On Mon, 2023-05-15 at 16:10 -0400, Olga Kornievskaia wrote:
> On Mon, May 15, 2023 at 2:58 PM Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Mon, 2023-05-15 at 11:26 -0700, dai.ngo@oracle.com wrote:
> > > On 5/15/23 11:14 AM, Olga Kornievskaia wrote:
> > > > On Sun, May 14, 2023 at 8:56 PM Dai Ngo <dai.ngo@oracle.com> wrote:
> > > > > If the GETATTR request on a file that has write delegation in effect
> > > > > and the request attributes include the change info and size attribute
> > > > > then the request is handled as below:
> > > > > 
> > > > > Server sends CB_GETATTR to client to get the latest change info and file
> > > > > size. If these values are the same as the server's cached values then
> > > > > the GETATTR proceeds as normal.
> > > > > 
> > > > > If either the change info or file size is different from the server's
> > > > > cached values, or the file was already marked as modified, then:
> > > > > 
> > > > >     . update time_modify and time_metadata into file's metadata
> > > > >       with current time
> > > > > 
> > > > >     . encode GETATTR as normal except the file size is encoded with
> > > > >       the value returned from CB_GETATTR
> > > > > 
> > > > >     . mark the file as modified
> > > > > 
> > > > > If the CB_GETATTR fails for any reasons, the delegation is recalled
> > > > > and NFS4ERR_DELAY is returned for the GETATTR.
> > > > Hi Dai,
> > > > 
> > > > I'm curious what does the server gain by implementing handling of
> > > > GETATTR with delegations? As far as I can tell it is not strictly
> > > > required by the RFC(s). When the file is being written any attempt at
> > > > querying its attribute is immediately stale.
> > > 
> > > Yes, you're right that handling of GETATTR with delegations is not
> > > required by the spec. The only benefit I see is that the server
> > > provides a more accurate state of the file as whether the file has
> > > been changed/updated since the client's last GETATTR. This allows
> > > the app on the client to take appropriate action (whatever that
> > > might be) when sharing files among multiple clients.
> > > 
> > 
> > 
> > 
> > From RFC 8881 10.4.3:
> > 
> > "It should be noted that the server is under no obligation to use
> > CB_GETATTR, and therefore the server MAY simply recall the delegation to
> > avoid its use."
> 
> This is a "MAY" which means the server can choose to not to and just
> return the info it currently has without recalling a delegation.
> 
> 

That's not at all how I read that. To me, it sounds like it's saying
that the only alternative to implementing CB_GETATTR is to recall the
delegation. If that's not the case, then we should clarify that in the
spec.

-- 
Jeff Layton <jlayton@kernel.org>