Subject: [nfsv4] cb_sequence referring calls list usage

From: Benny Halevy <[email-masked]>
Date: Mon, 03 Mar 2008 18:11:24 +0200

Now when layout stateids can be used by the client to
serialize layout get and return operations with respect
to cb_layoutrecall what is the use for csa_referring_call_lists?

Since it's quite complicated to implement and use on either
side of the wire I strongly suggest to git red of it if
possible.

Benny

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Benny Halevy <[email-masked]>
Date: Wed, 05 Mar 2008 12:47:14 +0200

I see it's used by CB_WANTS_CANCELLED so it needs to stay, *but*
Is the following still needed?

12.5.5.2.1.  Recall/Return Sequencing

   Section 2.10.5.3 describes the sessions mechanism for allowing the
   client to detect such situations in order to delay processing such a
   CB_LAYOUTRECALL.  The server MUST reference all conflicting LAYOUTGET
   operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.

Later, we correctly say:

   A client MUST NOT
   process a CB_LAYOUTRECALL that implies one or more outstanding
   LAYOUTGET or LAYOUTRETURN operations to which the client has not yet
   received a reply.  The client detects such a CB_LAYOUTRECALL by
   examining the "seqid" field of the recall's layout stateid.

If the seqid is sufficient for detecting the race why do we need
also the CB_SEQUENCE referring call lists mechanism for he same purpose?

It'd be a significant simplification if we rely solely on the layout stateid
to resolve the layout get/return/recall race.

If the client sent k layout get and return ops with seqid N
and the server processed m of them where m < k and then it
issued a cb_layoutrecall, the server will set lor_stateid's seqid
to N+m+1.  At this point, any layout get or return for this file
with seqid < N+m+1, and specifically the outstanding ops carrying
seqid==N are implicitly invalidated and the server MUST reject them
with NFS4ERR_BAD_STATEID.

The client, on its side, MUST wait until it receives all replies
with layout stateid.seqid in the range [N+1, N+m] before processing the
layout recall and sending LAYOUTRETURNs with seqid == N+m+1.
If the client times out (did not receive all replies within the lease
period(?) counting from(?)) it SHOULD return a NFS4ERR_DELAY error
to the layout recall callback.

Benny

---

From: "William A. (Andy) Adamson" <[email-masked]>
Date: Wed, 5 Mar 2008 10:08:06 -0500

I agree with Benny - In the CB_LAYOUTRECALL case, the stateid sequenceid is
enough to handle CB_LAYOUTRECALL and LAYOUTGET races. So  the use of  the
CB_SEQUENCE csa_referring_call_lists described in 12.5.5.2.1 is unnecessary
and should be removed.

-->Andy

---

From: [email-masked]
Date: Wed, 5 Mar 2008 14:01:59 -0500

I agree, and would like to point out a few nits....

Section 18.43.3

   If client sets loga_signal_layout_avail to TRUE, then it is
   registering with the client a "want" for a layout in the event the
   layout cannot be obtained due to resource exhaustion. 
 
s/with the client/with the server/

Section 1.6.3.2, last paragraph
                            NFSv4.1
   modifies named attributes relative to NFSv4.0 by tightening the
   allowed operations in order to prevent the development of non-
   interoperable implementation.  See Section 5.3
<http://tools.ietf.org/html/draft-ietf-nfsv4-minorversion1-21#section-5.
3>  for details.
 
s/implementation/implementations/

Section 20.7.3

   The argument, objects_to_keep means the total number of recallable
   objects of the types indicated in the argument type_mask that the
   server believes it can allow the client to have, including the number
   of such objects the client already has.  A client that tries to
   acquire more recallable objects than the server informs it can have
   runs the risk of having objects recalled.
 
s/informs it can have/informs it that it can have,/
Perhaps the sentence should be rewritten.

-Jason

________________________________

From: [email-masked] [mailto:[email-masked]] On Behalf
Of William A. (Andy) Adamson
Sent: Wednesday, March 05, 2008 10:08 AM
To: Benny Halevy
Cc: NFSv4
Subject: Re: [nfsv4] cb_sequence referring calls list usage

I agree with Benny - In the CB_LAYOUTRECALL case, the stateid sequenceid
is enough to handle CB_LAYOUTRECALL and LAYOUTGET races. So  the use of
the CB_SEQUENCE csa_referring_call_lists described in 12.5.5.2.1 is
unnecessary and should be removed.

-->Andy

On Wed, Mar 5, 2008 at 5:47 AM, Benny Halevy <[email-masked]>
wrote:

Later, we correctly say:

  A client MUST NOT

It'd be a significant simplification if we rely solely on the layout
stateid
to resolve the layout get/return/recall race.

Benny

---

From: Benny Halevy <[email-masked]>
Date: Wed, 12 Mar 2008 19:46:58 +0200

To allow the server not to use the referring call lists where possible
I propose the following change:

diff -Npu a/nfsv41_middle_pnfs.xml b/nfsv41_middle_pnfs.xml
--- a/nfsv41_middle_pnfs.xml
+++ b/nfsv41_middle_pnfs.xml
@@ -1011,12 +1011,14 @@
   <xref target="sessions_callback_races"/>
   describes the sessions mechanism for allowing the
   client to detect such situations in order to delay processing such a
-  CB_LAYOUTRECALL. The server MUST reference all conflicting LAYOUTGET
-  operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.  A
-  zero length array of referenced operations is used by the server to
-  tell the client that the server does not know of any LAYOUTGET
-  operations that conflict with the recall.
-
+  CB_LAYOUTRECALL. The server MAY reference all conflicting LAYOUTGET
+  operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.
+  An empty (or zero length) array of referenced operations is used by the
+  server to tell the client to wait on all outstanding LAYOUTGET and
+  LAYOUTRETURN operations until it receives all respective replies with
+  seqid less than the seqid given by CB_LAYOUTRECALL
+  (lor_stateid, see <xref target="OP_CB_LAYOUTRECALL"/>),
+  as described below.
 </t>
 <t>
   While referencing conflicting operations in CB_SEQUENCE

---

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 12 Mar 2008 16:12:29 -0400

It sounds like you are saying that if you have a non-empty list of
operations in the CB_SEQUENCE, then   it does not wait for the get and
return operations implied by the seqid.  I think it is just simpler to
not tie these together.  The seqid mchanism is always in effect and if
you specify operatiors to wait for in the CB_SEQUENCE (which people are
unlikely you to do), you wait for those as well. 

-----Original Message-----
From: Benny Halevy [mailto:[email-masked]] 
Sent: Wednesday, March 12, 2008 1:47 PM
To: NFSv4
Subject: Re: [nfsv4] cb_sequence referring calls list usage

To allow the server not to use the referring call lists where possible I
propose the following change:

diff -Npu a/nfsv41_middle_pnfs.xml b/nfsv41_middle_pnfs.xml
--- a/nfsv41_middle_pnfs.xml
+++ b/nfsv41_middle_pnfs.xml
@@ -1011,12 +1011,14 @@
   <xref target="sessions_callback_races"/>
   describes the sessions mechanism for allowing the
   client to detect such situations in order to delay processing such a
-  CB_LAYOUTRECALL. The server MUST reference all conflicting LAYOUTGET
-  operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.  A
-  zero length array of referenced operations is used by the server to
-  tell the client that the server does not know of any LAYOUTGET
-  operations that conflict with the recall.
-
+  CB_LAYOUTRECALL. The server MAY reference all conflicting LAYOUTGET  
+ operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.
+  An empty (or zero length) array of referenced operations is used by 
+ the  server to tell the client to wait on all outstanding LAYOUTGET 
+ and  LAYOUTRETURN operations until it receives all respective replies 
+ with  seqid less than the seqid given by CB_LAYOUTRECALL  
+ (lor_stateid, see <xref target="OP_CB_LAYOUTRECALL"/>),  as described 
+ below.
 </t>
 <t>
   While referencing conflicting operations in CB_SEQUENCE

On Mar. 05, 2008, 12:47 +0200, Benny Halevy <[email-masked]> wrote:
> I see it's used by CB_WANTS_CANCELLED so it needs to stay, *but* Is 
> the following still needed?
> 
> If the seqid is sufficient for detecting the race why do we need also 
> the CB_SEQUENCE referring call lists mechanism for he same purpose?
> 
> stateid to resolve the layout get/return/recall race.
> 
> If the client sent k layout get and return ops with seqid N and the 
> server processed m of them where m < k and then it issued a 
> cb_layoutrecall, the server will set lor_stateid's seqid to N+m+1.  At

> this point, any layout get or return for this file with seqid < N+m+1,

> and specifically the outstanding ops carrying seqid==N are implicitly 
> invalidated and the server MUST reject them with NFS4ERR_BAD_STATEID.
> 
> The client, on its side, MUST wait until it receives all replies with 
> layout stateid.seqid in the range [N+1, N+m] before processing the 
> period(?) counting from(?)) it SHOULD return a NFS4ERR_DELAY error to 
> the layout recall callback.
> 
> Benny
> 
>> Now when layout stateids can be used by the client to serialize 
>> layout get and return operations with respect to cb_layoutrecall what

>> is the use for csa_referring_call_lists?
>>
>> Since it's quite complicated to implement and use on either side of 
>> the wire I strongly suggest to git red of it if possible.
>>
>> Benny
>>

---

From: Benny Halevy <[email-masked]>
Date: Thu, 13 Mar 2008 13:29:27 +0200

> unlikely you to do), you wait for those as well. 

I agree.  Here's a new revision of the proposed text.  Note that I reversed
the order of the two paragraphs to start with the primary seqid mechanism.

Benny

diff -Npu a/nfsv41_middle_pnfs.xml b/nfsv41_middle_pnfs.xml
--- a/nfsv41_middle_pnfs.xml
+++ b/nfsv41_middle_pnfs.xml
@@ -1006,22 +1006,9 @@
   client's responsibility to properly serialize its layout requests
   and layout returns.
 </t>
-<section title="Recall/Return Sequencing">
+<section title="Layout Recall/Return Sequencing">
 <t>
-  <xref target="sessions_callback_races"/>
-  describes the sessions mechanism for allowing the
-  client to detect such situations in order to delay processing such a
-  CB_LAYOUTRECALL. The server MUST reference all conflicting LAYOUTGET
-  operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.  A
-  zero length array of referenced operations is used by the server to
-  tell the client that the server does not know of any LAYOUTGET
-  operations that conflict with the recall.
-
-</t>
-<t>
-  While referencing conflicting operations in CB_SEQUENCE
-  conveys to the client that the server is aware of
-  races, one critical issue with regard to operation sequencing
+  One critical issue with regard to layout operations sequencing
   concerns callbacks.  The protocol must defend against
   races between the reply to a LAYOUTGET or LAYOUTRETURN
   operation and a subsequent CB_LAYOUTRECALL.  A client
@@ -1033,9 +1020,25 @@
   is not one higher than what the client currently has recorded, and the
   client has at least one LAYOUTGET and/or LAYOUTRETURN
   operation outstanding, the client knows the server sent the CB_LAYOUTRECALL
-  after the server sent 
+  after sending
   a response to an outstanding LAYOUTGET or LAYOUTRETURN.
-
+  The client MUST wait before processing such a CB_LAYOUTRECALL
+  until it processes all replies for outstanding LAYOUTGET and
+  LAYOUTRETURN operations for the corresponding file
+  with seqid less than the seqid given by CB_LAYOUTRECALL
+  (lor_stateid, see <xref target="OP_CB_LAYOUTRECALL"/>.)
+</t>
+<t>
+  In addition to the seqid-based mechanism,
+  <xref target="sessions_callback_races"/>
+  describes the sessions mechanism for allowing the
+  client to detect callback race conditions and delay processing such a
+  CB_LAYOUTRECALL. The server MAY reference conflicting operations
+  in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.
+  The server has already sent replies for these operations before issuing
+  the callback and the replies may be racing with it.
+  The client MUST wait for all the referred calls to complete and update
+  its view of the layout state before processing the CB_LAYOUTRECALL.
 </t>
 
 <section title="Get/Return Sequencing">

---

From: [email-masked]
Date: Thu, 13 Mar 2008 12:15:26 -0400

I agree with the proposed text, but have two nits.  I find the following
sentence awkward.

> +  the callback and the replies may be racing with it.

"it" can easily be misread to refer to the server, which is confusing.
My preference would be to say... 

"Because the server has already sent replies for these operations before
issuing the callback, the replies may race with the CB_LAYOUTRECALL."

In the following sentence, replace "referred calls" with "referenced
calls" or "referring calls".  Either seems to be more consistent with
the rest of the spec.

> From: [email-masked] [mailto:[email-masked]] On Behalf
Of
> Benny Halevy
> Sent: Thursday, March 13, 2008 7:29 AM
> To: Noveck, Dave
> Cc: NFSv4
> >>    client to detect such situations in order to delay processing
such
> > a
also
> >> The client, on its side, MUST wait until it receives all replies
with
> >> period(?) counting from(?)) it SHOULD return a NFS4ERR_DELAY error
to

---

From: Benny Halevy <[email-masked]>
Date: Thu, 13 Mar 2008 18:34:12 +0200

> issuing the callback, the replies may race with the CB_LAYOUTRECALL."

Good point.

> calls" or "referring calls".  Either seems to be more consistent with

I like "referenced calls" a bit better, but I don't care much...

Thanks!

Benny

> such a

---

From: "Noveck, Dave" <[email-masked]>
Date: Thu, 13 Mar 2008 12:52:41 -0400

> I like "referenced calls" a bit better, but I don't care much...

I also think "referenced calls" works better.

-----Original Message-----
From: Benny Halevy [mailto:[email-masked]] 
Sent: Thursday, March 13, 2008 12:34 PM
To: [email-masked]
Cc: Noveck, Dave; [email-masked]
Subject: Re: [nfsv4] cb_sequence referring calls list usage

Good point.

Thanks!

Benny

>> On Mar. 12, 2008, 22:12 +0200, "Noveck, Dave"
<[email-masked]>
>> +  client to detect callback race conditions and delay processing
such

---

From: Mike Eisler <[email-masked]>
Date: Thu, 13 Mar 2008 19:05:38 -0700 (PDT)

I am declaring a rough consensus to make the changes Dave, Benny, and
Jason have been discussing. Attached are diffs.

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

> >>> operations in the CB_SEQUENCE, then   it does not wait for the
> get
> > and
> >>> return operations implied by the seqid.  I think it is just
> simpler
> > to
> >>> not tie these together.  The seqid mchanism is always in effect
> and
> > if
> >>> you specify operatiors to wait for in the CB_SEQUENCE (which
> people
> > are
> >> -  client to detect such situations in order to delay processing
> such
> > a
> >> -  operations in the CB_SEQUENCE that precedes the
> CB_LAYOUTRECALL.
> A
> >> -  zero length array of referenced operations is used by the
> server
> to
> >>    is not one higher than what the client currently has recorded,
> and
> > the
> >>> -  operations in the CB_SEQUENCE that precedes the
> CB_LAYOUTRECALL.
> > A
> >>> -  zero length array of referenced operations is used by the
> server
> > to
> >>> +  An empty (or zero length) array of referenced operations is
> used
> > by
> >>>>    Section 2.10.5.3 describes the sessions mechanism for
> allowing
> > the
> >>>>    LAYOUTGET or LAYOUTRETURN operations to which the client has
> not
> >>> yet
> >>>> cb_layoutrecall, the server will set lor_stateid's seqid to
> N+m+1.
> > At
> >>>> On Mar. 03, 2008, 18:11 +0200, Benny Halevy
> <[email-masked]>
> >>> wrote:
> >>>>> layout get and return operations with respect to
> cb_layoutrecall
> > what
> side
> > of
Index: nfsv41_middle_pnfs.xml
===================================================================
RCS file: /cvs/stdk/NFSv41/nfsv41_middle_pnfs.xml,v
retrieving revision 1.74
diff -u -r1.74 nfsv41_middle_pnfs.xml
--- nfsv41_middle_pnfs.xml	25 Feb 2008 20:51:37 -0000	1.74
+++ nfsv41_middle_pnfs.xml	14 Mar 2008 02:02:39 -0000
-<section title="Recall/Return Sequencing">
-<t>
-  <xref target="sessions_callback_races"/>
-  describes the sessions mechanism for allowing the
-  client to detect such situations in order to delay processing such a
-  CB_LAYOUTRECALL. The server MUST reference all conflicting LAYOUTGET
-  operations in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.  A
-  zero length array of referenced operations is used by the server to
-  tell the client that the server does not know of any LAYOUTGET
-  operations that conflict with the recall.
-
-</t>
+<section title="Layout Recall and Return Sequencing">
 <t>
-  While referencing conflicting operations in CB_SEQUENCE
-  conveys to the client that the server is aware of
-  races, one critical issue with regard to operation sequencing
+  One critical issue with regard to layout operations sequencing
   concerns callbacks.  The protocol must defend against
   races between the reply to a LAYOUTGET or LAYOUTRETURN
   operation and a subsequent CB_LAYOUTRECALL.  A client
@@ -1033,9 +1027,25 @@
   is not one higher than what the client currently has recorded, and the
   client has at least one LAYOUTGET and/or LAYOUTRETURN
   operation outstanding, the client knows the server sent the CB_LAYOUTRECALL
-  after the server sent 
+  after sending
   a response to an outstanding LAYOUTGET or LAYOUTRETURN.
-
+  The client MUST wait before processing such a CB_LAYOUTRECALL
+  until it processes all replies for outstanding LAYOUTGET and
+  LAYOUTRETURN operations for the corresponding file
+  with seqid less than the seqid given by CB_LAYOUTRECALL
+  (lor_stateid, see <xref target="OP_CB_LAYOUTRECALL"/>.)
+</t>
+<t>
+  In addition to the seqid-based mechanism,
+  <xref target="sessions_callback_races"/>
+  describes the sessions mechanism for allowing the
+  client to detect callback race conditions and delay processing such a
+  CB_LAYOUTRECALL. The server MAY reference conflicting operations
+  in the CB_SEQUENCE that precedes the CB_LAYOUTRECALL.
+  Because the server has already sent replies for these operations before
+  issuing the callback, the replies may race with the CB_LAYOUTRECALL.
+  The client MUST wait for all the referenced calls to complete and update
+  its view of the layout state before processing the CB_LAYOUTRECALL.
 </t>