Subject: [nfsv4] An issue regarding free byte-range lock state

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 26 Jun 2007 15:20:10 -0400

I'm re-roganizing the locking chapters and I came across the following
paragraph:
 
          Stateids must remain valid until either a client reboot or a 
          server reboot or until the client returns all of the locks 
          associated with the stateid by means of an operation such as
          CLOSE or DELEGRETURN.  If the locks are lost due to revocation
          the stateid remains a valid designation of that revoked state
until the 
          client frees it by using FREE_STATEID.  Stateids associated 
          with record locks are an exception.  They remain valid even 
          if a LOCKU frees all remaining locks, so long as the open file

          with which they are associated remains open, unless the client

          does a FREE_STATEID to cause the stateid to be freed.
 
It seems to me that RELEASE_LOCKOWNER should also free stateid's
associated with record locks (as long as there aren't any actual bytes
left 
locked).  Anybody see a problem with this?
_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 26 Jun 2007 15:45:30 -0400

On Tue, 2007-06-26 at 15:20 -0400, Noveck, Dave wrote:
>           the stateid remains a valid designation of that revoked
> state until the 

No, but I do have a question about the beginning of that paragraph: 
If stateids are supposed to remain valid until CLOSE time, does this
mean that the server is no longer supposed to increment the seqid field
in NFSv4.1 (see the top of page 23 in RFC3530)?

Trond

---

From: Mike Eisler <[email-masked]>
Date: Tue, 26 Jun 2007 12:26:04 -0700 (PDT)

Dave,

According to the spec, RELEASE_LOCKOWNER is not part of v4.1
"... because lock-owners with no associated locks
have any sequence-related state and so can be deleted by the
server at will."

--- "Noveck, Dave" <[email-masked]> wrote:

> state

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 26 Jun 2007 15:58:30 -0400

And the stateids could be freed individually if the client
desires, using FREE_STATEID.  Sounds OK to me.  

-----Original Message-----
From: Mike Eisler [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 3:26 PM
To: [email-masked]
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

Dave,

According to the spec, RELEASE_LOCKOWNER is not part of v4.1 "...
because lock-owners with no associated locks have any sequence-related
state and so can be deleted by the server at will."

> left locked).  Anybody see a problem with this?

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 26 Jun 2007 16:03:06 -0400

I think that still text allows the seqid to be incremented
with the stateid still considered valid.

Here's some relevant text about the server's choices on that:

       <t>
          In the case of stateids associated with opens,
          i.e. the stateids returned by OPEN (the state for
          the open, rather than that for the delegation),
          OPEN_DOWNGRADE, or CLOSE, the server MUST provide
          an "seqid" value starting at one for the first
          use of a given "other" value and incremented
          by one with each subsequent operation returning
          a stateid.
        </t>
        <t>
          In the case of other sorts of stateids (i.e. stateids
associated
          with record locks, layouts, and delegations), the 
          server MAY provide an
          incrementing sequence value on successive stateids returned
with
          same identifying field, or it may return the value zero.  If
it does
          return a non-zero "seqid" value it MUST start at one and be 
          incremented by one with each subsequent operation returning a 
          stateid with same
          "other" value, just as is done with open state.
        </t>

-----Original Message-----
From: Trond Myklebust [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 3:46 PM
To: Noveck, Dave
Cc: [email-masked]
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

Trond

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 26 Jun 2007 16:18:30 -0400

So when it says "Stateids must remain valid until..." does it mean that
the server is required to accept all stateids with 1 < seqid <= current
value and the correct opaque field, or should it still return
NFS4ERR_OLD_STATEID if the seqid field differs from the current value?

  Trond

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 26 Jun 2007 16:27:52 -0400

OLD_STATEID is gone.

The client should set seqid to 0 when sending stateid's.
The seqid is only a device whereby the cleint can determine
the order in which stateid's with the same opaque part 
were generated vy the server.

-----Original Message-----
From: Trond Myklebust [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 4:19 PM
To: Noveck, Dave
Cc: [email-masked]
Subject: RE: [nfsv4] An issue regarding free byte-range lock state

  Trond

On Tue, 2007-06-26 at 16:03 -0400, Noveck, Dave wrote:
> I think that still text allows the seqid to be incremented with the 
> stateid still considered valid.
> 

> mean that the server is no longer supposed to increment the seqid 
> field in NFSv4.1 (see the top of page 23 in RFC3530)?
> 
> Trond

---

From: Spencer Shepler <[email-masked]>
Date: Tue, 26 Jun 2007 15:47:55 -0500

> were generated vy the server.

This has been a source of confusion.

So, there is the seqid that is part of the various operations
and that certainly is supposed to be zero at all times.

Then there is stateid4.seqid.  This is not to be set to
zero by the client.  The client should not manipulate the
stateid in any way (just like a filehandle).  The stateid4.seqid
is exposed for the purpose you mention.

Old or previously stateids are not valid when a later
operation has changed the state, right?  For example, the
lock stateid as returned by LOCK, LOCKU refers to the
entire locking state of the file and when that state changes
because of a LOCK, LOCKU, the previous stateids returned
by the server are no longer valid because that particular
set of state no longer exists.

And which error is returned as part of that "old" stateid's use?

Spencer

---

From: Fredric Isaman <[email-masked]>
Date: Tue, 26 Jun 2007 16:58:44 -0400 (EDT)

> is exposed for the purpose you mention.

Um...just noting the spec specifically states otherwise.
>From 8.1.3.1 in draft-11:

Stateids are divided into two fields, a 96-bit "other" field
identifying the specific set of locks and a 32-bit "seqid" 
sequence value.
.
.
.
The client when using a stateid as a parameter to an operation, must,
except in the case of a special stateid, set the sequence value to
zero.  If the value is non-zero, the server MUST return the error
NFS4ERR_BAD_STATEID.

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 26 Jun 2007 17:10:19 -0400

If there is confusion, it is in draft-11, which says:

  7983	   The client when using a stateid as a parameter to an
operation, must,
  7984	   except in the case of a special stateid, set the sequence
value to
  7985	   zero.  If the value is non-zero, the server MUST return the
error
  7986	   NFS4ERR_BAD_STATEID.

I can't interpret that as saying anything other than what I said.

So if the client is supposed to keep the stateid seqid, what error 
should he get if he gets it wrong?  You would think OLD_STATEID
but draft-11 says OLD_STATEID is gone in v4.1. 

So it appears we need to come to some consensus about the seqid
field in stateid.  I thought we had, but apparently not.

-----Original Message-----
From: Spencer Shepler [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 4:48 PM
To: Noveck, Dave
Cc: Trond Myklebust; [email-masked]
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

On Jun 26, 2007, at 3:27 PM, Noveck, Dave wrote:

> The seqid is only a device whereby the cleint can determine the order 
> in which stateid's with the same opaque part were generated vy the 
> server.

So, there is the seqid that is part of the various operations and that
certainly is supposed to be zero at all times.

Then there is stateid4.seqid.  This is not to be set to zero by the
client.  The client should not manipulate the stateid in any way (just
like a filehandle).  The stateid4.seqid is exposed for the purpose you
mention.

Old or previously stateids are not valid when a later operation has
changed the state, right?  For example, the lock stateid as returned by
LOCK, LOCKU refers to the entire locking state of the file and when that
state changes because of a LOCK, LOCKU, the previous stateids returned
by the server are no longer valid because that particular set of state
no longer exists.

Spencer

> So when it says "Stateids must remain valid until..." does it mean 
> that the server is required to accept all stateids with 1 < seqid <= 
> current value and the correct opaque field, or should it still return

---

From: Rick Macklem <[email-masked]>
Date: Tue, 26 Jun 2007 17:25:18 -0400 (EDT)

> is exposed for the purpose you mention.
[stuff snipped]
> And which error is returned as part of that "old" stateid's use?
I thought that previous discussions w.r.t. NFS4ERR_OLDSTATEID had
agreed that the error was useless to a client and that a server
shouldn't care what value the stateid4.seqid field has, from experience
with 4.0?

Wearing my server implementor's hat:
- Didn't much matter, because it was just a couple of lines of code to
   check for/return NFS4ERR_OLDSTATEID vs ignore the field.

Wearing my client implementor's hat:
- If servers check for/return this error, Read/Write/Setattr_of_size Ops
   must be serialized with Open/Opendowngrade/Lock/LockU Ops on the same
   file. Just a bother and a performance hit, from what I can see. The
   only use the server has for the Stateid arg. in
   Read/Write/Setattr_of_size is to check for conflicting shares/locks
   (or more accurately, determining that a potentially conflicting
    share/lock is held by that client, so allowing the I/O is ok) and one
   with an out-of-date stateid4.seqid field should be fine for that?

At this point, I think a server should ignore the stateid4.seqid field
and never reply NFS4ERR_OLDSTATEID. (I can see an argument for checking/
returning the error for state changing Ops, but it just tells a client 
that it's busted and
hasn't serialized them properly. Maybe a debugging aid, but??)

rick

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 26 Jun 2007 17:28:46 -0400

>    with an out-of-date stateid4.seqid field should be fine for that?

Not really. As far as I know, most clients just resend using the new
stateid when we get an NFS4ERR_OLD_STATEID error on READ/WRITE/SETATTR.
Changes to the stateid are, after all, relatively rare occurrences.

> hasn't serialized them properly. Maybe a debugging aid, but??)

Isn't it useful on the _server_ in order to tell it when it needs to
fence off READ/WRITE requests after an OPEN_DOWNGRADE?

Trond

---

From: Spencer Shepler <[email-masked]>
Date: Tue, 26 Jun 2007 17:41:59 -0500

> fence off READ/WRITE requests after an OPEN_DOWNGRADE?

Yes.  The Solaris server uses the seqid as the value for
changing the stateid across the transitions of LOCK/LOCKU/ 
OPEN_DOWNGRADE, etc.

So, with this, the stateid is smaller for the server's content.

Spencer

---

From: Spencer Shepler <[email-masked]>
Date: Tue, 26 Jun 2007 17:44:43 -0500

> but draft-11 says OLD_STATEID is gone in v4.1.

BAD?  It is the wrong stateid.  BAD or OLD doesn't really change
the client's recovery.  If it is doing parallel opens, it will
check to see if there are other state changing ops for that filehandle
and will resend with the "latest" stateid based on the stateid.seqid.

> field in stateid.  I thought we had, but apparently not.

Yeah, too bad.  Leave it the way it was and remove the error as
discussed.  Less code on the client to zero and it is clearer
from a network trace point of view which stateid is which and
it expands the useful space for the server to store stateid info.

Spencer

>>>           incremented by one with each subsequent operation  
>>> returning
>> a
>>>           stateid with same
>>>>           Stateids must remain valid until either a client  
>>>> reboot or
>> a

---

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 27 Jun 2007 08:47:44 -0400

> > I can't interpret that as saying anything other than what I said.

Actually I don't see any confusion in draft-11.  I think it is what we
decided, for what appeared good reasons.  We can re-open the question,
of course.

> > should he get if he gets it wrong?  You would think OLD_STATEID but 
> > draft-11 says OLD_STATEID is gone in v4.1.

> BAD?  It is the wrong stateid.  

Not according to draft-11.  But if we do change things to require this,
these are different situations.

> BAD or OLD doesn't really change the client's recovery.  

In v4.0, BAD meant that the client or the server was out-to-lunch,
in some way.  OLD mean a need to re-issue with a new seqid.

> If it is doing parallel opens, it will check to see if there are 
> other state changing ops for that filehandle and will resend with 
> the "latest" stateid based on the stateid.seqid.

But for OPEN he doesn't know that when he issues the request.  Any
OPEN on the same fsid could potentially wind up opening a hard link
whose target is the same fh as the stateid.  So finding all outstanding
requests which potentially could update the seqid of the stateid is
complicated.

> > So it appears we need to come to some consensus about the seqid
field 
> > in stateid.  I thought we had, but apparently not.

> discussed.  

That's one opinion.  I vote for draft-11.  Other opinions?

> Less code on the client to zero and it is clearer from a network 
> trace point of view which stateid is which and it expands the useful 
> space for the server to store stateid info. 

I don't see that we are tight with stateid space, which is per-clientid.
We have two to the ninety-sixth for each client.

The big issue is that since the server knows what locks are associated,
having the client send the correct seqid doesn't help anything.  It 
just adds work.

Supposed I open a file and get stateid A,0.

Now I send an OPEN for write which (unknown to me) is for the same file 
and the server processes and sends back the stateid A,1 that I have not
got back yet.

Now suppose I send a READ with stateid A,0.  The server could process 
that since A designates the open file.  He looks at what rights I 
currently have, which include reading and lets me read.  Why is it
important that I know that I have seqid A,1 and thus know I could 
write?  Why should I reject the READ and force him to reissue it with
seqid A,1?  What's the benefit of forcing that?  I think draft-11 does
the right thing here in not forcing this.

We originally though aboutt getting rid of seqid in stateid but parallel
opens mean the client needs to be able to determine the order of 
stateid responses and draft-11 provides that, but there didn't seem 
to be any need for them when stateids are sent to the server.

-----Original Message-----
From: Spencer Shepler [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 6:45 PM
To: Noveck, Dave
Cc: [email-masked]; Trond Myklebust
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

On Jun 26, 2007, at 4:10 PM, Noveck, Dave wrote:

> should he get if he gets it wrong?  You would think OLD_STATEID but 
> draft-11 says OLD_STATEID is gone in v4.1.

BAD?  It is the wrong stateid.  BAD or OLD doesn't really change the
client's recovery.  If it is doing parallel opens, it will check to see
if there are other state changing ops for that filehandle and will
resend with the "latest" stateid based on the stateid.seqid.

>
> So it appears we need to come to some consensus about the seqid field 
> in stateid.  I thought we had, but apparently not.

Yeah, too bad.  Leave it the way it was and remove the error as
discussed.  Less code on the client to zero and it is clearer from a
network trace point of view which stateid is which and it expands the
useful space for the server to store stateid info.

Spencer

> changed the state, right?  For example, the lock stateid as returned 
> by LOCK, LOCKU refers to the entire locking state of the file and when

> that state changes because of a LOCK, LOCKU, the previous stateids 
> returned by the server are no longer valid because that particular set

> of state no longer exists.
>

>>>>           Stateids must remain valid until either a client reboot 
>>>> or
>> a
>>>>           if a LOCKU frees all remaining locks, so long as the open

>>>> file

---

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 27 Jun 2007 09:37:23 -0400

> > and never reply NFS4ERR_OLDSTATEID. (I can see an argument for 
> > checking/ returning the error for state changing Ops, but it just 
> > tells a client that it's busted and hasn't serialized them properly.

> > Maybe a debugging aid, but??)
> 
> Isn't it useful on the _server_ in order to tell it when it needs 
> to fence off READ/WRITE requests after an OPEN_DOWNGRADE? 

Not really.  Once I do the downngrade I'm not allowed to WRITE
(for example).  So the server makes sure I don't subsequently 
WRITE.  And I'm not allowed to write, whether I present a stateid
that was before from before the downgrade or after.  The thing
that needs to be checked are my actual rights and not whether I
have the most up-to-date stateid instance. 

-----Original Message-----
From: Trond Myklebust [mailto:[email-masked]] 
Sent: Tuesday, June 26, 2007 5:29 PM
To: Rick Macklem
Cc: Spencer Shepler; Noveck, Dave; [email-masked]
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

> and never reply NFS4ERR_OLDSTATEID. (I can see an argument for 
> checking/ returning the error for state changing Ops, but it just 
> tells a client that it's busted and hasn't serialized them properly. 
> Maybe a debugging aid, but??)

Trond

---

From: Trond Myklebust <[email-masked]>
Date: Wed, 27 Jun 2007 10:21:09 -0400

> have the most up-to-date stateid instance. 

I disagree. You could construe more elaborate scenarios such as

	OPEN_DOWNGRADE(READ)
	OPEN(WRITE)

In v4.0 any writes that were sent _before_ the OPEN_DOWNGRADE for read
are supposed to be fenced off by the server. I suppose, though, that in
v4.1 the session would appear to obsolete all that.

OK. I'm convinced that the seqid is not useful for the server, that
said, I do find it particularly ugly that the client should have to
modify a stateid before using it on the server.

Trond

---

From: Rick Macklem <[email-masked]>
Date: Wed, 27 Jun 2007 12:10:41 -0400 (EDT)

> Changes to the stateid are, after all, relatively rare occurrences.

This is my fallback approach if/when my client isn't serializing them.
(At this point, a BSD vnode lock serializes them as a part of the way
  the BSD VFS works, but the various BSDen are migrating away from this.
  My understanding of the Mac OS X vfs is that it already allows non-serial
  vops, based on a flag on the file system indicating it is safe to do
  so and FreeBSD seems to be headed the same direction.)

I don't really like the idea of "retry upon error" as the correct
implementation, but I can live with it.

> fence off READ/WRITE requests after an OPEN_DOWNGRADE?
Not for mine. (It checks the current open share permissions for each
read/write/setattr_of_size referenced by the stateid4.other.) But it
sounds like Solaris might, from Spencer's comment?

---

From: Garth Goodson <[email-masked]>
Date: Wed, 27 Jun 2007 11:01:34 -0700

> v4.1 the session would appear to obsolete all that.

What about with pNFS?  I don't think sessions help you in this case as 
there is potentially no ordering between the OPENs and READ/WRITE that 
is captured via sessions between the MDS and data servers; it seems that 
it must be captured within the stateid.

-Garth

---

From: Trond Myklebust <[email-masked]>
Date: Wed, 27 Jun 2007 14:31:52 -0400

> it must be captured within the stateid.

Good point!

Trond

---

From: "Noveck, Dave" <[email-masked]>
Date: Thu, 28 Jun 2007 09:36:05 -0400

> >> I'm not allowed to write, whether I present a stateid that was
before 
> >> from before the downgrade or after.  The thing that needs to be 
> >> checked are my actual rights and not whether I have the most 
> >> up-to-date stateid instance.
> >
> > are supposed to be fenced off by the server. I suppose, though, that

> > in

> What about with pNFS?  I don't think sessions help you in this case 
> as there is potentially no ordering between the OPENs and READ/WRITE 
> that is captured via sessions between the MDS and data servers; it 
> seems that it must be captured within the stateid. 

What specific case are you worried about, where Rick's approach
of checking the actual rights that someone has currently will do
something wrong?

Suppose I have an open for write with stateid (X,1).

And now I do a downgrade to read and that has stateid (X,2).

At this point the server must not allow writes, since some
other client/owner can do an open specifying deny=write, for 
example.

So given that the task is to reject writes, there are two 
ways to do that.  One is to ignore the sequence in the 
stateid, as Rick said.  The other is to check first if you
have a write with an old stateid, presumably issued before
the downgrade, and reject that the with stateid error.  Then
when the client retries, you still have to reject the write.
So what was the value of the stateid sequence check?  In the
end once the client presents the correct sequence, you have
to check the operation against the current rights, just the 
same.

-----Original Message-----
From: Goodson, Garth 
Sent: Wednesday, June 27, 2007 2:02 PM
To: Trond Myklebust
Cc: Noveck, Dave; Rick Macklem; [email-masked]; Spencer Shepler
Subject: Re: [nfsv4] An issue regarding free byte-range lock state

>> Not really.  Once I do the downngrade I'm not allowed to WRITE (for 
>> example).  So the server makes sure I don't subsequently WRITE.  And 
>> I'm not allowed to write, whether I present a stateid that was before

>> from before the downgrade or after.  The thing that needs to be 
>> checked are my actual rights and not whether I have the most 
>> up-to-date stateid instance.
> 

-Garth

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 28 Jun 2007 14:29:46 -0500

On Jun 26, 2007, at 2:20 PM, Noveck, Dave wrote:

> I'm re-roganizing the locking chapters and I came across the  
> following paragraph:
>
> associated with record locks (as long as there aren't any actual  
> bytes left

So, I would like to back up in this discussion a little.

I would like to come to a consensus of what stateid's represent
and how they do or do not change over the life time of the client's
and server's interaction.  I think once we have this agreed upon that
the specific issues of stateid.seqid handling will be fairly easy
to resolve.

In my reality, I am still of the mode that the stateid (in its entirety)
represents the current state of the file.  For the case of a client
and server, the state returned by either and OPEN or OPEN_DOWNGRADE must
be used for subsequent OPEN_DOWNGRADEs or CLOSEs.  The server may
change the stateid as a result of a OPEN_DOWNGRADE (not the same as
the value on the OPEN response).  However, the server is not required to
do this.

Using this model, the LOCK/LOCKU stateid changes also occur and
then each stateid represents the locking state at the point of
the completion of the LOCK/LOCKU.  Again, the server isn't required
to do this but it may do this if it wants.

I have seen the server's changing of the stateid a useful tool
to ensure the client and server have the correct/shared view
of their interaction (client returns what it received and the server
keep changing its value to help in overall correctness).

I assume given the thread on this topic that the alternate view
exists such that a stateid is assigned a value upon first instantiation
of state and it does/should/must remain the same until the sequence
of state events come to an end (via CLOSE, DELEGRETURN, FREE_STATEID)
as mentioned in the quoted text above.  True?

Spencer

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 28 Jun 2007 14:33:49 -0500

>   only use the server has for the Stateid arg. in

Most Unix clients serialize LOCK/LOCKU and READ/WRITE/SETATTR(size)  
because
of the expectation that modified data in the lock range is written
before LOCKU and cached data is released after a LOCK for the range.

Spencer

---

From: Rick Macklem <[email-masked]>
Date: Thu, 28 Jun 2007 17:48:33 -0400 (EDT)

On Thu, 28 Jun 2007, Spencer Shepler wrote:

>
> Most Unix clients serialize LOCK/LOCKU and READ/WRITE/SETATTR(size) because
Yes, to a point, my client does this (for a file handle):
- If LockU of a write locked region
   - flush writes to server before LockU

- If just got a Lock
   - flush stale data from the cache for the file handle

(I might call this partial serialization. It really only serializes LockU
  of a write locked region with Writes of that region and Lock with any
  Read/Write of that region. Setattr does currently get serialized.)
I believe this is sufficient to achieve the requirements of 3530 Sec. 
9.3.2. for advisory locking (knowing when mandatory locking is happenning
is a favourite topic of mine as you might recall:-) and unique file
handles.

What I don't serialize is Opens with Read/Write/Setattr_of_size. I do now
have code (recently added) that retries Read/Write/Setattr_of_size if I 
get NFS4ERR_OLDSTATEID back from the server. (The case Dave described with
respect to Opening a different hard link to the same file could result in
this for 4.0.)

rick