[nfsv4] Re: question around CB_GETATTR handling

[Rick Macklem <[e-mail]>]

On Wed, Sep 4, 2024 at 1:09 PM Jeff Layton <[e-mail]> wrote:
>
> On Wed, 2024-09-04 at 19:47 +0000, Trond Myklebust wrote:
> > On Wed, 2024-09-04 at 15:33 -0400, Jeff Layton wrote:
> > > On Wed, 2024-09-04 at 19:21 +0000, Trond Myklebust wrote:
> > > > On Wed, 2024-09-04 at 12:23 -0400, Jeff Layton wrote:
> > > > > RFC 8881, section 10.4.3 has this pseudo code to describe how to
> > > > > handle
> > > > > the size and change attr:
> > > > >
> > > > > ------------------8<-------------------
> > > > >     if (!modified) {
> > > > >         do CB_GETATTR for change and size;
> > > > >
> > > > >         if (cc != sc)
> > > > >             modified = TRUE;
> > > > >     } else {
> > > > >         do CB_GETATTR for size;
> > > > >     }
> > > > >
> > > > >     if (modified) {
> > > > >         sc = sc + 1;
> > > > >         time_modify = time_metadata = current_time;
> > > > >         update sc, time_modify, time_metadata into file's
> > > > > metadata;
> > > > >     }
> > > > > ------------------8<-------------------
> > > > >
> > > > >
> > > > > The line that shows sc = sc + 1 seems to indicate that we need to
> > > > > increment "sc" on every CB_GETATTR, even if nothing changed since
> > > > > the
> > > > > last time.
> > > > >
> > > > > Is that correct or should we only increment it if it wasn't
> > > > > incremented
> > > > > before? The latter would make more sense, given that it doesn't
> > > > > query
> > > > > for change if the file is already considered to be modified.
> > > >
> > > > I believe RFC5661 says
> > > >
> > > >    For simplicity of implementation, the client MAY for each
> > > > CB_GETATTR
> > > >    return the same value d.  This is true even if, between
> > > > successive
> > > >    CB_GETATTR operations, the client again modifies the file's data
> > > > or
> > > >    metadata in its cache.  The client can return the same value
> > > > because
> > > >    the only requirement is that the client be able to indicate to
> > > > the
> > > >    server that the client holds modified data.  Therefore, the
> > > > value of
> > > >    d may always be c + 1.
> > > >
> > > > Which is how the Linux client tries to implement it.
> > > >
> > >
> > > Right, 8881 is similar.
> >
> > Yeah, but nobody has coded to that.
> >
> > >
> > > In this case though, I'm asking about the server-side implementation.
> > > That has its own algorithm (illustrated above in the pseudocode), and
> > > it seems to indicate that once the server detects that the client has
> > > made a single modification to the file, that it needs to increment
> > > "sc"
> > > on every subsequent CB_GETATTR.
> > >
> > > That makes no sense on its face, so I suspect this is a mistake in
> > > the
> > > pseudocode above. Or am I wrong and this is actually necessary?
> > > --
> >
> > Oh... No, as far as the server is concerned, I think it is indeed
> > supposed to increment the change attribute and timestamps on every
> > reply. That is reinforced here:
> >
> >    As discussed earlier in this section, the client MAY return the same
> >    cc value on subsequent CB_GETATTR calls, even if the file was
> >    modified in the client's cache yet again between successive
> >    CB_GETATTR calls.  Therefore, the server must assume that the file
> >    has been modified yet again, and MUST take care to ensure that the
> >    new nsc it constructs and returns is greater than the previous nsc it
> >    returned.
> >
> > The spec also says that "committing the constructed attribute values to
> > stable storage is OPTIONAL". That seems misleading or possibly just
> > incorrect. The fact that the client declares that it is caching data
> > doesn't necessarily mean that it will actually manage to write that
> > data back.
I think the server does have the option of not saving the change attribute
on stable storage and then, after a server reboot, if the client does a
OPEN/CLAIM_PREVIOUS for the delegation, the server can require
an immediate recall of the delegation. (By setting recall == true in the
open_read_delegation4/open_write_delegation4.)

At least I think that works?

>
> Huh, ok.
>
> So imagine that the client gets a write deleg and makes a single small
> change to the file, but then holds on to the write deleg for a long
> time. Other clients are issuing CB_GETATTRs against the server.
> Eventually the delegation is returned.
>
> Could you end up in a change attr reuse situation due to all of this
> faux change attr incrementing by the server?
Since change is 64bits, I think this will take a very very long time?

rick

> --
> Jeff Layton <[e-mail]>
>
> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]