Subject: [nfsv4] [PATCH] layout-sim: layout recall callback (forgetful client)

From: Benny Halevy <[email-masked]>
Date: Thu, 27 Jan 2011 17:06:06 +0200

Simulate cb_layoutrecall with the forget client model.
The client responds with NFS4ERR_DELAY if there are any calls it needs
to wait for before processing the callback, or if there I/Os in flight
(simulated randomally).  When the layout can be returned, the client
deleted it locally and replies synchronously with NFS4ERR_NOMATCHING_LAYOUT
instead of sending layout return.

The server sends only one CB_LAYOUT_RECALL at a time and does not process
any LAYOUTGETs while waiting on it, replying with NFS4ERR_RECALLCONFLICT.
LAYOUTRETURN stateid sequencing is enforced so that only those sent after
the client processed the layout recall callback are accepted.
The layout recall "process" is completed either on NFS4ERR_RECALLCONFLICT
or on a matching LAYOUTRETURN.

Signed-off-by: Benny Halevy <[email-masked]>
---
 client.c |   37 ++++++++++++++++++++++--
 layout.c |   21 +++++++++----
 layout.h |   18 +++++++++++-
 server.c |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 150 insertions(+), 19 deletions(-)

diff --git a/client.c b/client.c
index 61163b5..182db7b 100644
--- a/client.c
+++ b/client.c
@@ -171,6 +171,8 @@ _reprocess_queue(void)
 	struct msg *m;
 
 	list_for_each_entry (m, &cl_queued.list, list) {
+		if (m->type == CB_LAYOUT_RECALL)
+			continue;
 		if (is_next(&m->stateid, &cl_layout.stateid) ||
 		    (m->type == LAYOUTRETURN &&
 		     m->stateid.type == OPEN_STATEID &&
@@ -227,6 +229,36 @@ out_free:
 }
 
 static void
+cl_recv_cb_layout_recall(struct msg *m)
+{
+	if ((m->stateid.type == OPEN_STATEID &&
+	     cl_layout.stateid.type != OPEN_STATEID) ||
+	    (m->stateid.seq > cl_layout.stateid.seq + 1)) {
+		dprintk(1, "%s: m=%p seq=%llu cl_layout.seq=%llu: waiting\n",
+			__func__, m,
+			llu(m->stateid.seq), llu(cl_layout.stateid.seq));
+		m->status = NFS4ERR_DELAY;
+		goto out;
+	}
+
+	BUG_ON(m->stateid.seq <= cl_layout.stateid.seq);
+	if (mynrand(10) < 3) {
+		dprintk(1, "%s: m=%p: simulating delay\n", __func__, m);
+		m->status = NFS4ERR_DELAY;
+		goto out;
+	}
+
+	/* forgetful model */
+	layout_delete(&cl_layout, &m->range);
+	cl_layout.stateid = m->stateid;
+	m->status = NFS4ERR_NOMATCHING_LAYOUT;
+
+out:
+	dprintk(1, "%s: m=%p status=%d\n", __func__, m, m->status);
+	enqueue(&srv_msg_queue, m);
+}
+
+static void
 _cl_recv_msg(struct msg *m)
 {
 	switch (m->type) {
@@ -236,10 +268,9 @@ _cl_recv_msg(struct msg *m)
 	case LAYOUTRETURN:
 		cl_recv_layoutreturn(m);
 		break;
-#if 0
-	case CB_LAYOUTRECALL:
+	case CB_LAYOUT_RECALL:
+		cl_recv_cb_layout_recall(m);
 		break;
-#endif
 	default:
 		BUG_ON(true);
 	}
diff --git a/layout.c b/layout.c
index 94870f0..c5ad99a 100644
--- a/layout.c
+++ b/layout.c
@@ -65,7 +65,7 @@ last_byte_offset(u64 start, u64 len)
  *                  |----------------|
  */
 static inline int
-lo_seg_overlapping(struct layout_range *l1, struct layout_range *l2)
+lo_seg_overlapping(const struct layout_range *l1, const struct layout_range *l2)
 {
 	u64 start1 = l1->offset;
 	u64 last1 = last_byte_offset(start1, l1->length);
@@ -82,13 +82,15 @@ lo_seg_overlapping(struct layout_range *l1, struct layout_range *l2)
 }
 
 bool
-layout_delete(struct layout *lo, struct layout_range *range)
+_lookup_layout(const char *func, struct layout *lo,
+	       const struct layout_range *range,
+	       bool do_delete)
 {
 	bool found = false;
 	struct layout_item *pos, *next;
 
 	dprintk(1, "%s: %s: offset=%llu length=%llu iomode=%d roc=%d\n",
-		__func__, lo->desc,
+		func, lo->desc,
 		llu(range->offset), llu(range->length),
 		range->iomode, range->roc);
 
@@ -98,9 +100,12 @@ layout_delete(struct layout *lo, struct layout_range *range)
 		    (!range->roc || pos->range.roc) &&
 		    lo_seg_overlapping(&pos->range, range)) {
 			dprintk(1, "%s: found l=%p\n", __func__, pos);
-			list_del(&pos->list);
-			myfree(pos);
 			found = true;
+			if (do_delete) {
+				list_del(&pos->list);
+				myfree(pos);
+			} else
+				break;
 		}
 	}
 
@@ -113,8 +118,10 @@ layout_verify(struct layout *server, struct layout *client)
 	bool ok = true, found;
 	struct layout_item *cl, *sl;
 
-	if ((client->stateid.type != server->stateid.type) ||
-	    (client->stateid.seq != server->stateid.seq)) {
+	if (((client->stateid.type != server->stateid.type) ||
+	     (client->stateid.seq != server->stateid.seq)) &&
+	    (!server->layout_recall_msg ||
+	     client->stateid.seq >= server->stateid.seq)) {
 		dprintk(0, "%s: cl stateid=%d:%llu srv stateid=%d:%llu\n",
 			__func__,
 			client->stateid.type, llu(client->stateid.seq),
diff --git a/layout.h b/layout.h
index bafad13..895b145 100644
--- a/layout.h
+++ b/layout.h
@@ -43,15 +43,31 @@ struct stateid {
 		    type resets to OPEN_STATEID */
 };
 
+struct msg;
+
 struct layout {
 	struct stateid stateid;
 	struct list_head segs;
 	char *desc;
+	struct msg *layout_recall_msg;
 };
 
 void layout_init(struct layout *, char *desc);
 void layout_insert(struct layout *, struct layout_range *);
-bool layout_delete(struct layout *, struct layout_range *);
+bool _lookup_layout(const char *func, struct layout *,
+		    const struct layout_range *,
+		    bool do_delete);
+
+static inline bool has_layout(struct layout *lo, const struct layout_range *range)
+{
+	return _lookup_layout(__func__, lo, range, false);
+}
+
+static inline bool layout_delete(struct layout *lo, const struct layout_range *range)
+{
+	return _lookup_layout(__func__, lo, range, true);
+}
+
 bool layout_verify(struct layout *server, struct layout *client);
 
 enum msg_type {
diff --git a/server.c b/server.c
index 1bc9978..19bb85a 100644
--- a/server.c
+++ b/server.c
@@ -5,6 +5,7 @@
  * See "COPYING" for more information
  */
 
+#include "alloc.h"
 #include "client.h"
 #include "debug.h"
 #include "layout.h"
@@ -63,10 +64,13 @@ verify_stateid(struct msg *m)
 		    srv_layout.stateid.seq > 100) {
 			/* FIXME: use an actual limit on parallelism */
 			return m->status = NFS4ERR_OLD_STATEID;
-		} else if (m->stateid.type == LAYOUT_STATEID &&
-		           m->stateid.seq > srv_layout.stateid.seq)
-			return m->status = NFS4ERR_BAD_STATEID;
-		/* TODO: CB_LAYOUTRECALL case */
+		} else if (m->stateid.type == LAYOUT_STATEID) {
+			if (srv_layout.layout_recall_msg &&
+		            m->stateid.seq < srv_layout.layout_recall_msg->stateid.seq)
+				return m->status = NFS4ERR_OLD_STATEID;
+			else if (m->stateid.seq > srv_layout.stateid.seq)
+				return m->status = NFS4ERR_BAD_STATEID;
+		}
 		break;
 	default:
 		BUG_ON(true);
@@ -77,8 +81,13 @@ verify_stateid(struct msg *m)
 static void
 srv_recv_layoutget(struct msg *m)
 {
+	if (srv_layout.layout_recall_msg) {
+		m->status = NFS4ERR_RECALLCONFLICT;
+		goto out;
+	}
+
 	if (verify_stateid(m))
-		return;
+		goto out;
 
 	/* randomize layoutget response */
 	m->range.length = BLOCK_SIZE + mynrand(MAX_LO_MSG_LENGTH - BLOCK_SIZE);
@@ -87,22 +96,54 @@ srv_recv_layoutget(struct msg *m)
 		m->range.iomode = IOMODE_RW;
 	srv_set_stateid(m);
 	m->status = NFS4_OK;
-	dump_msg(m);
 	layout_insert(&srv_layout, &m->range);
+out:
+	dump_msg(m);
 }
 
 static void
 srv_recv_layoutreturn(struct msg *m)
 {
+	struct msg *lrm;
+
 	if (verify_stateid(m))
 		return;
 
 	layout_delete(&srv_layout, &m->range);
 	srv_set_stateid(m);
 	m->status = NFS4_OK;
+	lrm = srv_layout.layout_recall_msg;
+	if (lrm && m->range.iomode == lrm->range.iomode &&
+	    m->range.offset == lrm->range.offset &&
+	    m->range.length && lrm->range.length) {
+		srv_layout.layout_recall_msg = NULL;
+		myfree(lrm);
+	}
 	dump_msg(m);
 }
 
+static void
+srv_recv_cb_layout_recall(struct msg *m)
+{
+	BUG_ON(m != srv_layout.layout_recall_msg);
+	dump_msg(m);
+	switch (m->status) {
+	case NFS4_OK:
+	case NFS4ERR_DELAY:
+		dprintk(1, "%s: m=%p: layout_recall resent\n",
+			__func__, srv_layout.layout_recall_msg);
+		enqueue(&cl_msg_queue, srv_layout.layout_recall_msg);
+		break;
+	case NFS4ERR_NOMATCHING_LAYOUT:
+		layout_delete(&cl_layout, &m->range);
+		srv_layout.layout_recall_msg = NULL;
+		myfree(m);
+		break;
+	default:
+		BUG_ON(true);
+	}
+}
+
 void
 srv_recv_msg(void)
 {
@@ -121,15 +162,51 @@ srv_recv_msg(void)
 	case LAYOUTRETURN:
 		srv_recv_layoutreturn(m);
 		break;
+	case CB_LAYOUT_RECALL:
+		srv_recv_cb_layout_recall(m);
+		return;
 	default:
 		BUG_ON(true);
 	}
 	enqueue(&cl_msg_queue, m);
 }
 
+static void
+srv_recall_layout(void)
+{
+	struct msg *m;
+
+	if (srv_layout.layout_recall_msg) {
+		dprintk(1, "%s: m=%p layout_recall already in progress: status=%d\n",
+			__func__,
+			srv_layout.layout_recall_msg,
+			srv_layout.layout_recall_msg->status);
+		return;
+	}
+
+	dprintk(1, "%s\n", __func__);
+	m = myzalloc(sizeof(*m));
+	m->type = CB_LAYOUT_RECALL;
+	m->range.offset = mynrand(MAX_LO_MSG_OFFSET) & ~BLOCK_MASK;
+	m->range.length = BLOCK_SIZE + mynrand(MAX_LO_MSG_LENGTH - BLOCK_SIZE);
+	m->range.iomode = IOMODE_READ + mynrand(3);
+	/* bump server stateid.seq */
+	srv_layout.stateid.seq++;
+	m->stateid =  srv_layout.stateid;
+	m->status = -1;
+	srv_layout.layout_recall_msg = m;
+	dump_msg(m);
+
+	enqueue(&cl_msg_queue, m);
+}
+
+struct rand_event srv_events[] = {
+	{ 75, srv_recv_msg },
+	{ 25, srv_recall_layout },
+};
+
 void
 srv_draw_event(void)
 {
-	srv_recv_msg();
+	rand_call(srv_events);
 }
-
-- 
1.7.3.4

---

From: <[email-masked]>
Date: Thu, 27 Jan 2011 20:44:11 -0500

I have a couple questions about this whole area.

By this patch, you've made yourself the expert about it.  Sorry about that.  I owe you a drink.  You can collect at FAST.

The first question is about section 12.5.5.2.1.3 which discusses RECALLCONFLICT.  It discusses the case of an overlapping LAYOUTGET arriving, but you are assuming that the text applies to all LAYOUTGETs for the same file.  Comments?  If it does apply to all, then the word "overlapping" should not be there.  On the other hand, if it doesn't, then it might be nice for the spec to make this clear.  Is there some way in which you are depending on rejecting all LAYOUTGETs, rather than just the overlapping ones?  I can't see one but the spec does overlapping layouts only.

When the LAYOUTRECALL is re-issued, I'm assuming the seqid is incremented. Is this true?

Another question is about your statement "the server only sends one CB_LAYOUTRECALL at a time and does not process any LAYOUTGETs while waiting on it."  Is the server considered "waiting on it" after it has received the DELAY but before re-issuing it.  Not sure 12.5.5.2.1.3 gives a clear answer to the issue or any answer but I'd liked to know your view.

Another interesting issue is the definition of "forgetful model".  This term is not defined in the spec.  Section 12.5.3 use the phrase "using the forgetful model" but does supply a definition.  It makes reference to section 12.5.5.1 which merely says, "It may be useful for clients to 'forget' details about what layouts and ranges the client actually has", but seems to treat this as an optional/transient situation.  I'm given to understand that your interpretation of a client "using the forgetful model" is one who will never ever do a LAYOUTRETURN, i.e. one who is determined never to find it useful.  Is that your understanding?

But now let's get back to section 12.5.3.  Suppose you get a LAYOUTGET response and you process it using the "forgetful model" exception and so you respond before adding those layout segments to your list.  Then a CB_LAYOUTRECALL arrives and you process and you process it before adding those segments to your list and so respond saying you don't have them.  So the server thinks you don't have them and you do, which is not supposed to happen.  Am I missing something here, or is this "forgetful model" exception just wrong?

The more I look at it, the more I think 12.5.5.2.1.3 needs to be rewritten:

  * It isn't all that clear about overlapping LAYOUTGET issue.

  * It doesn't deal with the case of the a LAYOUTRECALL being 
    terminated by either DELAY or NOMATCHING_LAYOUT.

  * It is structured around seqid but it would seem that it 
    applies to the ALL and FSID cases in which there is no 
    seqid for the recall and it needs to be made clear how
    this all works in these cases.  Also "same file" is not
    appropriate for this case.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Benny Halevy
Sent: Thursday, January 27, 2011 10:06 AM
To: [email-masked]; [email-masked]
Cc: Benny Halevy
Subject: [nfsv4] [PATCH] layout-sim: layout recall callback (forgetful client)

 
 
 static void
 
 bool

 

+struct msg;
+
 struct layout {
 	struct stateid stateid;
 	struct list_head segs;
 	char *desc;

 enum msg_type {
 
+#include "alloc.h"
 #include "client.h"
 #include "debug.h"
 #include "layout.h"

 static void

+static void
 
+static void
+srv_recall_layout(void)
+{
+	struct msg *m;
+

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

---

From: Fred Isaman <[email-masked]>
Date: Thu, 27 Jan 2011 22:16:22 -0500

> By this patch, you've made yourself the expert about it.  Sorry about that.  I owe you a drink.  You can collect at FAST.

I'm also interested in hearing Benny's opinion, but since I've just
implemented this part of the spec, here are my answers.

> The first question is about section 12.5.5.2.1.3 which discusses RECALLCONFLICT.  It discusses the case of an overlapping LAYOUTGET arriving, but you are assuming that the text applies to all LAYOU…

I've think overlapping is important, though I don't know how Benny's
simulator is interpreting this.

> When the LAYOUTRECALL is re-issued, I'm assuming the seqid is incremented. Is this true?

I think it has to be incremented, otherwise there is no way for the
client to know which LAYOUTRETURNS have been processed at the server
between the first CB_LAYOUTRECALL and its reissue.

> Another question is about your statement "the server only sends one CB_LAYOUTRECALL at a time and does not process any LAYOUTGETs while waiting on it."  Is the server considered "waiting on it" aft…

I also don't think the spec is clear, but I have been working on the
assumption that the server has no further obligations (to the client)
once it receives DELAY from the client.  Obviously, it is likely to
reissue the CB_LAYOUTRECALL or unilaterally revoke the layout, but for
all the client knows the server could just decide it doesn't need the
layout back anymore and never send another CB_LAYOUTRECALL.

> Another interesting issue is the definition of "forgetful model".  This term is not defined in the spec.  Section 12.5.3 use the phrase "using the forgetful model" but does supply a definition.  It…

While that is what the linux client does, I tend to think of
"forgetful model" more as the client has the option at any point, as
long as it is not using some range of a layout,  to wipe from its
memory any knowledge of that range, possibly including the entire
layout and its stateid, without notifying the server that it has done
so.

> But now let's get back to section 12.5.3.  Suppose you get a LAYOUTGET response and you process it using the "forgetful model" exception and so you respond before adding those layout segments to your list.

I agree your interpretation leads to nonsense.  I read 12.5.3 as if
   For LAYOUTGET results, if the client is not using the
   forgetful model (Section 12.5.5.1), it MUST first update its record
   of what ranges of the file's layout it has before using the seqid.

were replaced with
   For LAYOUTGET results, it MUST first update its record of those ranges
   from the returned layout it decides to use (see forgetful model
Section 12.5.5.1)
   before using the seqid.

Fred

> Then a CB_LAYOUTRECALL arrives and you process and you process it before adding those segments to your list and so respond saying you don't have them.  So the server thinks you don't have them and you do, which is not supposed to happen.  Am I missing something here, or is this "forgetful model" exception just wrong?
>

---

From: Benny Halevy <[email-masked]>
Date: Mon, 31 Jan 2011 15:16:06 +0200

On 2011-01-28 05:16, Fred Isaman wrote:
>> By this patch, you've made yourself the expert about it.  Sorry
>> about that.  I owe you a drink.  You can collect at FAST.
>> 
> 
>> The first question is about section 12.5.5.2.1.3 which discusses
>> RECALLCONFLICT.  It discusses the case of an overlapping LAYOUTGET
>> arriving, but you are assuming that the text applies to all
>> LAYOUTGETs for the same file.  Comments?  

The simulator currently simplifies that, as you noticed, but my interpretation
was that rejecting all LAYOUTGETs is safe as it is inclusive of any overlapping
LAYOUTGETs.  I did not assume that implementation is what the text meant or should
mean.

>> If it does apply to all,
>> then the word "overlapping" should not be there.  On the other
>> hand, if it doesn't, then it might be nice for the spec to make
>> this clear.  Is there some way in which you are depending on
>> rejecting all LAYOUTGETs, rather than just the overlapping ones?

I certainly hope not.  As a TODO item I'll just add the overlap
condition to the simulator just to make sure.

>> I can't see one but the spec does overlapping layouts only.
>> 

Agreed.

> between the first CB_LAYOUTRECALL and its reissue.

Good point.
The simulator currently re-sends the original message with the same seq
but it needs to be updated to reflect the state at re-issue time.

> 
>> Another question is about your statement "the server only sends one
>> CB_LAYOUTRECALL at a time and does not process any LAYOUTGETs while
>> waiting on it."  Is the server considered "waiting on it" after it
>> has received the DELAY but before re-issuing it.
>>  Not sure
>> 12.5.5.2.1.3 gives a clear answer to the issue or any answer but
>> I'd liked to know your view.
>> 

That's what the simulator does, just to get the simple straight forward case out sooner.
A real server might choose to behave differently.

One could wonder if NFS4ERR_RECALLCONFLICT is the right error to return in this case
as there is no actual callback in flight but I think that the spec's definition
for NFS4ERR_RECALLCONFLICT can cater for this case, as well as case 3 in
12.5.5.2.1.3, in which even when the server receives a successful reply to CB_LAYOUTRECALL
(not a NFS4ERR_DELAY error), it still replies with NFS4ERR_RECALLCONFLICT until
the callback operation (I'd like to call it a meta-operation to differentiate it
from the RPC level operation) is completed by receiving the terminating LAYOUTRETURN
associated with the CB_LAYOUTRECALL.

> assumption that the server has no further obligations (to the
> client) once it receives DELAY from the client.  Obviously, it is
> likely to reissue the CB_LAYOUTRECALL or unilaterally revoke the
> layout, but for all the client knows the server could just decide it
> doesn't need the layout back anymore and never send another
> CB_LAYOUTRECALL.
> 
> 
>> Another interesting issue is the definition of "forgetful model".
>> This term is not defined in the spec.  Section 12.5.3 use the
>> phrase "using the forgetful model" but does supply a definition.
>> It makes reference to section 12.5.5.1 which merely says, "It may
>> be useful for clients to 'forget' details about what layouts and
>> ranges the client actually has", but seems to treat this as an
>> optional/transient situation.  I'm given to understand that your
>> interpretation of a client "using the forgetful model" is one who
>> will never ever do a LAYOUTRETURN, i.e. one who is determined never
>> to find it useful.  Is that your understanding?
>> 

> layout and its stateid, without notifying the server that it has
> done so.
> 
> 

Right.  I do see forget clients using LAYOUTRETURN in some cases,
e.g., reporting errors with the objects layout type, or responding
to CB_RECALL_ANY.  Currently, in the return_on_close case, until
specified differently, the client is expected to send a LAYOUTRETURN
before CLOSE and there's is no elegant way to get around it right now.
Trond suggested to specify that CLOSE will implicitly return the layout
if the server marked it (or parts of it, my addition) as "return_on_close".

>> But now let's get back to section 12.5.3.  Suppose you get a
>> LAYOUTGET response and you process it using the "forgetful model"
>> exception and so you respond before adding those layout segments to
>> your list.

The client must actually forget it at this point and never ever cache it,
otherwise it didn't really forget the layout but rather it just had a
transient memory failure :)

> For LAYOUTGET results, if the client is not using the forgetful model
> (Section 12.5.5.1), it MUST first update its record of what ranges of
> the file's layout it has before using the seqid.
> 

Agrees.

> were replaced with For LAYOUTGET results, it MUST first update its
> record of those ranges from the returned layout it decides to use
> (see forgetful model Section 12.5.5.1) before using the seqid.
> 
> 
> Fred
> 
>> Then a CB_LAYOUTRECALL arrives and you process and you process it
>> before adding those segments to your list and so respond saying you
>> don't have them.  So the server thinks you don't have them and you
>> do, which is not supposed to happen.  Am I missing somethinatg here,
>> or is this "forgetful model" exception just wrong?
>> 

Yes, this needs to be clarifies, in particular the NOMATCHING_LAYOUT
case and what is the seq after it.  And with that in mind what happens
when the client has no layout for the file after erring with NOMATCHING_LAYOUT.
Two cases:
A. CB_LAYOUTRECALL was for IOMODE_ANY, offset=0, length=~0.
   in this case it is very clear for both the server and the client that
   there is no layout and the stateid can be safely reset, equivalently to
   a LAYOUTRETURN with lrs_present=false

B. CB_LAYOUTRECALL for a partial range or with iomode != IOMODE_ANY.
   I'd like to avoid a chance that the client and server would go out of sync.
   - In case we define (in the errata) that the client can send a LAYOUTGET
   with a non-layout stateid at any time (something it MUST NOT according to
   the current definition after establishing the layout state and before
   receiving a LAYOUTRETURN response with lrs_present=false), and if the client
   thinks it has no layout and it has no outstanding LAYOUTGETs it can just
   reset its view of the stateid unilaterally, assuming that any LAYOUTGET
   will use the initial stateid thus it will tell the server implicitly that the
   client might have forgot the layout before hand.  The server, on the other hand
   MUST allow the client to continue to use the previous stateid "lineage"
   so it can't forget it.
   - That said, this model might be unnecessarily complicated and we can just
   decide that the client and server maintain the layout state normally.

C. An exception for B can be defined if iomode != IOMODE_ANY and the client
   has ever got layouts of this iomode and only it and the range is the whole file,
   in this case the stateid can be safely managed the same as in case A.

>> 
>> * It is structured around seqid but it would seem that it applies
>> to the ALL and FSID cases in which there is no seqid for the recall
>> and it needs to be made clear how this all works in these cases.
>> Also "same file" is not appropriate for this case.

True.

Benny

>> 
>> 
>> 
>> -----Original Message----- From: [email-masked]
>> [mailto:[email-masked]] On Behalf Of Benny Halevy Sent:
>> Thursday, January 27, 2011 10:06 AM To: [email-masked];
>> [email-masked] Cc: Benny Halevy Subject: [nfsv4] [PATCH]
>> layout-sim: layout recall callback (forgetful client)
>> 
>> Simulate cb_layoutrecall with the forget client model. The client
>> responds with NFS4ERR_DELAY if there are any calls it needs to wait
>> for before processing the callback, or if there I/Os in flight 
>> (simulated randomally).  When the layout can be returned, the
>> client deleted it locally and replies synchronously with
>> NFS4ERR_NOMATCHING_LAYOUT instead of sending layout return.
>> 
>> The server sends only one CB_LAYOUT_RECALL at a time and does not
>> process any LAYOUTGETs while waiting on it, replying with
>> NFS4ERR_RECALLCONFLICT. LAYOUTRETURN stateid sequencing is enforced
>> so that only those sent after the client processed the layout
>> recall callback are accepted. The layout recall "process" is
>> completed either on NFS4ERR_RECALLCONFLICT or on a matching
>> LAYOUTRETURN.
>> 
>> Signed-off-by: Benny Halevy <[email-masked]> --- client.c |
>> 37 ++++++++++++++++++++++-- layout.c |   21 +++++++++---- layout.h
>> |   18 +++++++++++- server.c |   93
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 4
>> files changed, 150 insertions(+), 19 deletions(-)
>> 
>> diff --git a/client.c b/client.c index 61163b5..182db7b 100644 ---
>> a/client.c +++ b/client.c @@ -171,6 +171,8 @@
>> _reprocess_queue(void) struct msg *m;
>> 
>> list_for_each_entry (m, &cl_queued.list, list) { +               if
>> (m->type == CB_LAYOUT_RECALL) +                       continue; if
>> (is_next(&m->stateid, &cl_layout.stateid) || (m->type ==
>> LAYOUTRETURN && m->stateid.type == OPEN_STATEID && @@ -227,6
>> +229,36 @@ out_free: }
>> 
>> static void +cl_recv_cb_layout_recall(struct msg *m) +{ +       if
>> ((m->stateid.type == OPEN_STATEID && +
>> cl_layout.stateid.type != OPEN_STATEID) || +
>> (m->stateid.seq > cl_layout.stateid.seq + 1)) { +
>> dprintk(1, "%s: m=%p seq=%llu cl_layout.seq=%llu: waiting\n", +
>> __func__, m, +                       llu(m->stateid.seq),
>> llu(cl_layout.stateid.seq)); +               m->status =
>> NFS4ERR_DELAY; +               goto out; +       } + +
>> BUG_ON(m->stateid.seq <= cl_layout.stateid.seq); +       if
>> (mynrand(10) < 3) { +               dprintk(1, "%s: m=%p:
>> simulating delay\n", __func__, m); +               m->status =
>> NFS4ERR_DELAY; +               goto out; +       } + +       /*
>> forgetful model */ +       layout_delete(&cl_layout, &m->range); +
>> cl_layout.stateid = m->stateid; +       m->status =
>> NFS4ERR_NOMATCHING_LAYOUT; + +out: +       dprintk(1, "%s: m=%p
>> status=%d\n", __func__, m, m->status); +
>> enqueue(&srv_msg_queue, m); +} + +static void _cl_recv_msg(struct
>> msg *m) { switch (m->type) { @@ -236,10 +268,9 @@
>> _cl_recv_msg(struct msg *m) case LAYOUTRETURN: 
>> cl_recv_layoutreturn(m); break; -#if 0 -       case
>> CB_LAYOUTRECALL: +       case CB_LAYOUT_RECALL: +
>> cl_recv_cb_layout_recall(m); break; -#endif default: BUG_ON(true); 
>> } diff --git a/layout.c b/layout.c index 94870f0..c5ad99a 100644 
>> --- a/layout.c +++ b/layout.c @@ -65,7 +65,7 @@
>> last_byte_offset(u64 start, u64 len) *
>> |----------------| */ static inline int -lo_seg_overlapping(struct
>> layout_range *l1, struct layout_range *l2) 
>> +lo_seg_overlapping(const struct layout_range *l1, const struct
>> layout_range *l2) { u64 start1 = l1->offset; u64 last1 =
>> last_byte_offset(start1, l1->length); @@ -82,13 +82,15 @@
>> lo_seg_overlapping(struct layout_range *l1, struct layout_range
>> *l2) }
>> 
>> bool -layout_delete(struct layout *lo, struct layout_range *range) 
>> +_lookup_layout(const char *func, struct layout *lo, +
>> const struct layout_range *range, +              bool do_delete) { 
>> bool found = false; struct layout_item *pos, *next;
>> 
>> dprintk(1, "%s: %s: offset=%llu length=%llu iomode=%d roc=%d\n", -
>> __func__, lo->desc, +               func, lo->desc, 
>> llu(range->offset), llu(range->length), range->iomode,
>> range->roc);
>> 
>> @@ -98,9 +100,12 @@ layout_delete(struct layout *lo, struct
>> layout_range *range) (!range->roc || pos->range.roc) && 
>> lo_seg_overlapping(&pos->range, range)) { dprintk(1, "%s: found
>> l=%p\n", __func__, pos); -
>> list_del(&pos->list); -                       myfree(pos); found =
>> true; +                       if (do_delete) { +
>> list_del(&pos->list); +                               myfree(pos); 
>> +                       } else +
>> break; } }
>> 
>> @@ -113,8 +118,10 @@ layout_verify(struct layout *server, struct
>> layout *client) bool ok = true, found; struct layout_item *cl,
>> *sl;
>> 
>> -       if ((client->stateid.type != server->stateid.type) || -
>> (client->stateid.seq != server->stateid.seq)) { +       if
>> (((client->stateid.type != server->stateid.type) || +
>> (client->stateid.seq != server->stateid.seq)) && +
>> (!server->layout_recall_msg || +            client->stateid.seq >=
>> server->stateid.seq)) { dprintk(0, "%s: cl stateid=%d:%llu srv
>> stateid=%d:%llu\n", __func__, client->stateid.type,
>> llu(client->stateid.seq), diff --git a/layout.h b/layout.h index
>> bafad13..895b145 100644 --- a/layout.h +++ b/layout.h @@ -43,15
>> +43,31 @@ struct stateid { type resets to OPEN_STATEID */ };
>> 
>> +struct msg; + struct layout { struct stateid stateid; struct
>> list_head segs; char *desc; +       struct msg *layout_recall_msg; 
>> };
>> 
>> void layout_init(struct layout *, char *desc); void
>> layout_insert(struct layout *, struct layout_range *); -bool
>> layout_delete(struct layout *, struct layout_range *); +bool
>> _lookup_layout(const char *func, struct layout *, +
>> const struct layout_range *, +                   bool do_delete); 
>> + +static inline bool has_layout(struct layout *lo, const struct
>> layout_range *range) +{ +       return _lookup_layout(__func__, lo,
>> range, false); +} + +static inline bool layout_delete(struct layout
>> *lo, const struct layout_range *range) +{ +       return
>> _lookup_layout(__func__, lo, range, true); +} + bool
>> layout_verify(struct layout *server, struct layout *client);
>> 
>> enum msg_type { diff --git a/server.c b/server.c index
>> 1bc9978..19bb85a 100644 --- a/server.c +++ b/server.c @@ -5,6 +5,7
>> @@ * See "COPYING" for more information */
>> 
>> +#include "alloc.h" #include "client.h" #include "debug.h" #include
>> "layout.h" @@ -63,10 +64,13 @@ verify_stateid(struct msg *m) 
>> srv_layout.stateid.seq > 100) { /* FIXME: use an actual limit on
>> parallelism */ return m->status = NFS4ERR_OLD_STATEID; -
>> } else if (m->stateid.type == LAYOUT_STATEID && -
>> m->stateid.seq > srv_layout.stateid.seq) -
>> return m->status = NFS4ERR_BAD_STATEID; -               /* TODO:
>> CB_LAYOUTRECALL case */ +               } else if (m->stateid.type
>> == LAYOUT_STATEID) { +                       if
>> (srv_layout.layout_recall_msg && +
>> m->stateid.seq < srv_layout.layout_recall_msg->stateid.seq) +
>> return m->status = NFS4ERR_OLD_STATEID; +
>> else if (m->stateid.seq > srv_layout.stateid.seq) +
>> return m->status = NFS4ERR_BAD_STATEID; +               } break; 
>> default: BUG_ON(true); @@ -77,8 +81,13 @@ verify_stateid(struct msg
>> *m) static void srv_recv_layoutget(struct msg *m) { +       if
>> (srv_layout.layout_recall_msg) { +               m->status =
>> NFS4ERR_RECALLCONFLICT; +               goto out; +       } + if
>> (verify_stateid(m)) -               return; +               goto
>> out;
>> 
>> /* randomize layoutget response */ m->range.length = BLOCK_SIZE +
>> mynrand(MAX_LO_MSG_LENGTH - BLOCK_SIZE); @@ -87,22 +96,54 @@
>> srv_recv_layoutget(struct msg *m) m->range.iomode = IOMODE_RW; 
>> srv_set_stateid(m); m->status = NFS4_OK; -       dump_msg(m); 
>> layout_insert(&srv_layout, &m->range); +out: +       dump_msg(m); 
>> }
>> 
>> static void srv_recv_layoutreturn(struct msg *m) { +       struct
>> msg *lrm; + if (verify_stateid(m)) return;
>> 
>> layout_delete(&srv_layout, &m->range); srv_set_stateid(m); 
>> m->status = NFS4_OK; +       lrm = srv_layout.layout_recall_msg; +
>> if (lrm && m->range.iomode == lrm->range.iomode && +
>> m->range.offset == lrm->range.offset && +           m->range.length
>> && lrm->range.length) { +
>> srv_layout.layout_recall_msg = NULL; +               myfree(lrm); +
>> } dump_msg(m); }
>> 
>> +static void +srv_recv_cb_layout_recall(struct msg *m) +{ +
>> BUG_ON(m != srv_layout.layout_recall_msg); +       dump_msg(m); +
>> switch (m->status) { +       case NFS4_OK: +       case
>> NFS4ERR_DELAY: +               dprintk(1, "%s: m=%p: layout_recall
>> resent\n", +                       __func__,
>> srv_layout.layout_recall_msg); +
>> enqueue(&cl_msg_queue, srv_layout.layout_recall_msg); +
>> break; +       case NFS4ERR_NOMATCHING_LAYOUT: +
>> layout_delete(&cl_layout, &m->range); +
>> srv_layout.layout_recall_msg = NULL; +               myfree(m); +
>> break; +       default: +               BUG_ON(true); +       } +} 
>> + void srv_recv_msg(void) { @@ -121,15 +162,51 @@
>> srv_recv_msg(void) case LAYOUTRETURN: srv_recv_layoutreturn(m); 
>> break; +       case CB_LAYOUT_RECALL: +
>> srv_recv_cb_layout_recall(m); +               return; default: 
>> BUG_ON(true); } enqueue(&cl_msg_queue, m); }
>> 
>> +static void +srv_recall_layout(void) +{ +       struct msg *m; + +
>> if (srv_layout.layout_recall_msg) { +               dprintk(1, "%s:
>> m=%p layout_recall already in progress: status=%d\n", +
>> __func__, +                       srv_layout.layout_recall_msg, +
>> srv_layout.layout_recall_msg->status); +               return; +
>> } + +       dprintk(1, "%s\n", __func__); +       m =
>> myzalloc(sizeof(*m)); +       m->type = CB_LAYOUT_RECALL; +
>> m->range.offset = mynrand(MAX_LO_MSG_OFFSET) & ~BLOCK_MASK; +
>> m->range.length = BLOCK_SIZE + mynrand(MAX_LO_MSG_LENGTH -
>> BLOCK_SIZE); +       m->range.iomode = IOMODE_READ + mynrand(3); +
>> /* bump server stateid.seq */ +       srv_layout.stateid.seq++; +
>> m->stateid =  srv_layout.stateid; +       m->status = -1; +
>> srv_layout.layout_recall_msg = m; +       dump_msg(m); + +
>> enqueue(&cl_msg_queue, m); +} + +struct rand_event srv_events[] =
>> { +       { 75, srv_recv_msg }, +       { 25, srv_recall_layout }, 
>> +}; + void srv_draw_event(void) { -       srv_recv_msg(); +
>> rand_call(srv_events); } - -- 1.7.3.4
>> 
>> _______________________________________________ nfsv4 mailing list 
>> [email-masked] https://www.ietf.org/mailman/listinfo/nfsv4
>> 
>> _______________________________________________ nfsv4 mailing list 
>> [email-masked] https://www.ietf.org/mailman/listinfo/nfsv4
>>

---

From: Benny Halevy <[email-masked]>
Date: Mon, 31 Jan 2011 15:42:13 +0200

>>> Also "same file" is not appropriate for this case.

12.5.5.2.1.5 does actually define this as follows:

   In the event the client sends a LAYOUTGET or
   LAYOUTRETURN while a bulk recall is in progress, the server returns
   NFS4ERR_RECALLCONFLICT

> True.

One more issue is that I think is not covered is server handling of LAYOUTRETURN
with a seq older than the CB_LAYOUTRECALL's one.
The simulator return NFS4ERR_OLD_STATEID in this case which is safe to do.

---

From: <[email-masked]>
Date: Mon, 31 Jan 2011 16:19:34 -0500

> forgetful model" more as the client has the option at any point, as

But once you get past section 12.5.5.1, every client has that option.

That what I'm calling the "normal-memory model" (Hey, sometimes clients forget things; nobody's perfect).

I think the basic point is that we are in agreement that statements that you don't have to do something if you are using "the forgetful model" are dubious because have an option to do something or having done it some number of finite times does not guarantee your future behavior sufficiently to make something OK that wouldn't be OK otherwise.  Or at least if it does, the spec needs a careful definition of what that would mean.

I'll be sending a document out tomorrow that lays out some choices for clearing up this and other pNFS clarity/consistency/ambiguity issues

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Fred Isaman
Sent: Thursday, January 27, 2011 10:16 PM
To: Noveck, David
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: Re: [nfsv4] [PATCH] layout-sim: layout recall callback (forgetful client)

were replaced with

Fred