[PATCH 0/9] pnfsd fixes

[PATCH 0/9] pnfsd fixes

[Benny Halevy]

[PATCH 1/9] SQUASHME: pnfsd: fix lrs_present calculation
[PATCH 2/9] SQUASHME: pnfsd: return_on_close all layout segments
[PATCH 3/9] SQUASHME: pnfsd: do not dequeue layout state on
[PATCH 4/9] SQUASHME: pnfsd: remove fs_layout_return sb parameter
[PATCH 5/9] SQUASHME: pnfsd: fix locking around fs_layout_return
[PATCH 6/9] pnfsd: LR_FLAG_CL_EMPTY layout return flag
[PATCH 7/9] pnfsd: LR_FLAG_EMPTY layout return flag
[PATCH 8/9] SQUASHME: pnfsd: Something very wrong with
[PATCH 9/9] SQUASHME: pnfsd: no use for fi_layout_states list

[PATCH 1/9] SQUASHME: pnfsd: fix lrs_present calculation

[Benny Halevy]

lrs_present must be set to zero iff the client holds no layout state on
the respective file.

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 3931e52..5c50d8a 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -893,7 +893,7 @@ struct super_block *
 	struct nfs4_layout *lp, *nextlp;
 
 	dprintk("%s: clp %p fp %p\n", __func__, clp, fp);
-	lrp->lrs_present = 1;
+	lrp->lrs_present = 0;
 	spin_lock(&layout_lock);
 	list_for_each_entry_safe (lp, nextlp, &fp->fi_layouts, lo_perfile) {
 		dprintk("%s: lp %p client %p,%p lo_type %x,%x iomode %d,%d\n",
@@ -901,19 +901,22 @@ struct super_block *
 			lp->lo_client, clp,
 			lp->lo_seg.layout_type, lrp->args.lr_seg.layout_type,
 			lp->lo_seg.iomode, lrp->args.lr_seg.iomode);
-		if (lp->lo_client != clp ||
-		    lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type ||
+		if (lp->lo_client != clp)
+			continue;
+		if (lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type ||
 		    (lp->lo_seg.iomode != lrp->args.lr_seg.iomode &&
 		     lrp->args.lr_seg.iomode != IOMODE_ANY) ||
-		     !lo_seg_overlapping(&lp->lo_seg, &lrp->args.lr_seg))
+		     !lo_seg_overlapping(&lp->lo_seg, &lrp->args.lr_seg)) {
+			lrp->lrs_present = 1;
 			continue;
+		}
 		layouts_found++;
 		trim_layout(&lp->lo_seg, &lrp->args.lr_seg);
 		if (!lp->lo_seg.length) {
-			lrp->lrs_present = 0;
 			dequeue_layout(lp);
 			destroy_layout(lp);
-		}
+		} else
+			lrp->lrs_present = 1;
 	}
 	if (ls && layouts_found && lrp->lrs_present)
 		update_layout_stateid_locked(ls, &lrp->lr_sid);
-- 
1.7.11.7

[PATCH 2/9] SQUASHME: pnfsd: return_on_close all layout segments

[Benny Halevy]

As per errata #3226
http://www.rfc-editor.org/errata_search.php?rfc=5661
return_on_close refers to all segments acquired by the client
not only those marked with logr_return_on_close

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 5c50d8a..fe3f693 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1242,7 +1242,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
 		bool empty;
 
 		/* Check for a match */
-		if (!lo->lo_state->ls_roc || lo->lo_client != clp)
+		if (lo->lo_client != clp)
 			continue;
 
 		/* Return the layout */
-- 
1.7.11.7

[PATCH 3/9] SQUASHME: pnfsd: do not dequeue layout state on destroy_layout

[Benny Halevy]

the layout state may be shared by a number of layout segments,
each held by struct nfs4_layout and we may be destroying just one of them
e.g. on layoutreturn.

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index fe3f693..c62af3d 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -345,7 +345,6 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
 		__func__, lp, clp, fp, fp->fi_inode);
 
 	kmem_cache_free(pnfs_layout_slab, lp);
-	list_del_init(&ls->ls_perfile);
 	/* release references taken by init_layout */
 	put_layout_state(ls);
 	put_nfs4_file(fp);
@@ -1249,6 +1248,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
 		memset(&lr, 0, sizeof(lr));
 		lr.args.lr_return_type = RETURN_FILE;
 		lr.args.lr_seg = lo->lo_seg;
+		list_del_init(&lo->lo_state->ls_perfile);	/* just to be on the safe side */
 		dequeue_layout(lo);
 		destroy_layout(lo); /* do not access lp after this */
 
@@ -1301,6 +1301,7 @@ void pnfs_expire_client(struct nfs4_client *clp)
 			lr.args.lr_seg = lp->lo_seg;
 			empty = list_empty(&lp->lo_file->fi_layouts);
 			BUG_ON(lp->lo_client != clp);
+			list_del_init(&lp->lo_state->ls_perfile);	/* just to be on the safe side */
 			dequeue_layout(lp);
 			destroy_layout(lp); /* do not access lp after this */
 		}
-- 
1.7.11.7

[PATCH 4/9] SQUASHME: pnfsd: remove fs_layout_return sb parameter

[Benny Halevy]

From: Boaz Harrosh <bharrosh@panasas.com>

Derive the superblock from the inode argument.
Let the caller provide the right inode, rather than NULL
for RETURN_{FSID,ALL}

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index c62af3d..b8d0db8 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -350,11 +350,12 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
 	put_nfs4_file(fp);
 }
 
-void fs_layout_return(struct super_block *sb, struct inode *ino,
-		      struct nfsd4_pnfs_layoutreturn *lrp, int flags,
-		      void *recall_cookie)
+static void fs_layout_return(struct inode *ino,
+			     struct nfsd4_pnfs_layoutreturn *lrp,
+			     int flags, void *recall_cookie)
 {
 	int ret;
+	struct super_block *sb = ino->i_sb;
 
 	if (unlikely(!sb->s_pnfs_op->layout_return))
 		return;
@@ -362,9 +363,6 @@ void fs_layout_return(struct super_block *sb, struct inode *ino,
 	lrp->args.lr_flags = flags;
 	lrp->args.lr_cookie = recall_cookie;
 
-	if (!ino) /* FSID or ALL */
-		ino = sb->s_root->d_inode;
-
 	ret = sb->s_pnfs_op->layout_return(ino, &lrp->args);
 	dprintk("%s: inode %lu iomode=%d offset=0x%llx length=0x%llx "
 		"cookie = %p flags 0x%x status=%d\n",
@@ -1081,7 +1079,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	nfs4_unlock_state();
 
 	/* call exported filesystem layout_return (ignore return-code) */
-	fs_layout_return(sb, ino, lrp, 0, recall_cookie);
+	fs_layout_return(ino, lrp, 0, recall_cookie);
 
 out_no_fs_call:
 	dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1201,9 +1199,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 		inode = igrab(clr->clr_file->fi_inode);
 		if (WARN_ON(!inode))
 			return;
-	} else {
-		inode = NULL;
-	}
+	} else
+		inode = clr->clr_sb->s_root->d_inode;
 
 	dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
 		clr->clr_client, clr->clr_file);
@@ -1219,8 +1216,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	recall_cookie = layoutrecall_done(clr);
 	spin_unlock(&layout_lock);
 
-	fs_layout_return(clr->clr_sb, inode, &lr, LR_FLAG_INTERN,
-			 recall_cookie);
+	fs_layout_return(inode, &lr, LR_FLAG_INTERN, recall_cookie);
 	iput(inode);
 }
 
@@ -1255,8 +1251,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
 		empty = list_empty(&fp->fi_layouts);
 		found = true;
 		dprintk("%s: fp=%p clp=%p: return on close", __func__, fp, clp);
-		fs_layout_return(fp->fi_inode->i_sb, fp->fi_inode, &lr,
-				 LR_FLAG_INTERN,
+		fs_layout_return(fp->fi_inode, &lr, LR_FLAG_INTERN,
 				 empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
 	}
 	spin_unlock(&layout_lock);
@@ -1315,7 +1310,7 @@ void pnfs_expire_client(struct nfs4_client *clp)
 		dprintk("%s: inode %lu lp %p clp %p\n", __func__, inode->i_ino,
 			lp, clp);
 
-		fs_layout_return(inode->i_sb, inode, &lr, LR_FLAG_EXPIRE,
+		fs_layout_return(inode, &lr, LR_FLAG_EXPIRE,
 				 empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
 		iput(inode);
 	}
-- 
1.7.11.7

[PATCH 5/9] SQUASHME: pnfsd: fix locking around fs_layout_return

[Benny Halevy]

From: Boaz Harrosh <bharrosh@panasas.com>

Every code path that eventually called fs_layout_return
had different locking. Some held both the layout_spinlock
has well as nfs-lock. Some one or the other, some none.
Fix all sites to never hold any locks, before calling
The FS.

Also, deprecate PNFS_LAST_LAYOUT_NO_RECALLS.
To be replaced with lr_flags bit

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c             | 176 ++++++++++++++++++++++------------------
 include/linux/nfsd/nfsd4_pnfs.h |   2 -
 2 files changed, 97 insertions(+), 81 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index b8d0db8..d48abba 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -186,6 +186,9 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
 	kfree(ls);
 }
 
+/*
+ * Note: caller must not hold layout_lock
+ */
 static void
 put_layout_state(struct nfs4_layout_state *ls)
 {
@@ -321,6 +324,9 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
 	dprintk("pNFS %s end\n", __func__);
 }
 
+/*
+ * Note: always called under the layout_lock
+ */
 static void
 dequeue_layout(struct nfs4_layout *lp)
 {
@@ -329,7 +335,7 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
 }
 
 /*
- * Note: always called under the layout_lock
+ * Note: caller must not hold layout_lock
  */
 static void
 destroy_layout(struct nfs4_layout *lp)
@@ -881,10 +887,59 @@ struct super_block *
 	dprintk("%s:End lo %llu:%lld\n", __func__, lo->offset, lo->length);
 }
 
+static void
+pnfsd_return_lo_list(struct list_head *lo_destroy_list, struct inode *ino_orig,
+		     struct nfsd4_pnfs_layoutreturn *lr_orig, int flags,
+		     void *cb_cookie)
+{
+	struct nfs4_layout *lo, *nextlp;
+
+	if (list_empty(lo_destroy_list) && cb_cookie) {
+		/* This is a rare race case where at the time of recall there
+		 * were some layouts, which got freed before the recall-done.
+		 * and the recall was left without any actual layouts to free.
+		 * If the FS gave us a cb_cookie it is waiting for it so we
+		 * report back about it here.
+		 */
+
+		/* Any caller with a cb_cookie must pass a none null
+		 * ino_orig and an lr_orig.
+		 */
+		struct inode *inode = igrab(ino_orig);
+
+		/* Probably a BUG_ON but just in case. Caller of cb_recall must
+		 * take care of this. Please report to ml */
+		if (WARN_ON(!inode))
+			return;
+
+		fs_layout_return(inode, lr_orig, flags, cb_cookie);
+		iput(inode);
+		return;
+	}
+
+	list_for_each_entry_safe(lo, nextlp, lo_destroy_list, lo_perfile) {
+		struct inode *inode = lo->lo_file->fi_inode;
+		struct nfsd4_pnfs_layoutreturn lr;
+		bool empty;
+
+		memset(&lr, 0, sizeof(lr));
+		lr.args.lr_return_type = RETURN_FILE;
+		lr.args.lr_seg = lo->lo_seg;
+
+		list_del(&lo->lo_perfile);
+		empty = list_empty(lo_destroy_list);
+
+		fs_layout_return(inode, &lr, flags, empty ? cb_cookie : NULL);
+
+		destroy_layout(lo); /* this will put the lo_file */
+	}
+}
+
 static int
 pnfs_return_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp,
 			 struct nfsd4_pnfs_layoutreturn *lrp,
-			 struct nfs4_layout_state *ls)
+			 struct nfs4_layout_state *ls,
+			 struct list_head *lo_destroy_list)
 {
 	int layouts_found = 0;
 	struct nfs4_layout *lp, *nextlp;
@@ -911,7 +966,7 @@ struct super_block *
 		trim_layout(&lp->lo_seg, &lrp->args.lr_seg);
 		if (!lp->lo_seg.length) {
 			dequeue_layout(lp);
-			destroy_layout(lp);
+			list_add_tail(&lp->lo_perfile, lo_destroy_list);
 		} else
 			lrp->lrs_present = 1;
 	}
@@ -924,7 +979,8 @@ struct super_block *
 
 static int
 pnfs_return_client_layouts(struct nfs4_client *clp,
-			   struct nfsd4_pnfs_layoutreturn *lrp, u64 ex_fsid)
+			   struct nfsd4_pnfs_layoutreturn *lrp, u64 ex_fsid,
+			   struct list_head *lo_destroy_list)
 {
 	int layouts_found = 0;
 	struct nfs4_layout *lp, *nextlp;
@@ -942,7 +998,7 @@ struct super_block *
 
 		layouts_found++;
 		dequeue_layout(lp);
-		destroy_layout(lp);
+		list_add_tail(&lp->lo_perfile, lo_destroy_list);
 	}
 	spin_unlock(&layout_lock);
 
@@ -1004,10 +1060,10 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	struct inode *ino = current_fh->fh_dentry->d_inode;
 	struct nfs4_file *fp = NULL;
 	struct nfs4_client *clp;
-	struct nfs4_layout_state *ls = NULL;
 	struct nfs4_layoutrecall *clr, *nextclr;
 	u64 ex_fsid = current_fh->fh_export->ex_fsid;
 	void *recall_cookie = NULL;
+	LIST_HEAD(lo_destroy_list);
 
 	dprintk("NFSD: %s\n", __func__);
 
@@ -1017,6 +1073,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 		goto out;
 
 	if (lrp->args.lr_return_type == RETURN_FILE) {
+		struct nfs4_layout_state *ls = NULL;
+
 		fp = find_file(ino);
 		if (!fp) {
 			nfs4_unlock_state();
@@ -1037,12 +1095,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 			goto out_put_file;
 
 		/* update layouts */
-		layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls);
-		/* optimize for the all-empty case */
-		if (list_empty(&fp->fi_layouts))
-			recall_cookie = PNFS_LAST_LAYOUT_NO_RECALLS;
+		layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls,
+							 &lo_destroy_list);
+		put_layout_state(ls);
 	} else {
-		layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid);
+		layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid,
+							   &lo_destroy_list);
 	}
 
 	dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1073,13 +1131,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 out_put_file:
 	if (fp)
 		put_nfs4_file(fp);
-	if (ls)
-		put_layout_state(ls);
+
 out:
 	nfs4_unlock_state();
 
-	/* call exported filesystem layout_return (ignore return-code) */
-	fs_layout_return(ino, lrp, 0, recall_cookie);
+	pnfsd_return_lo_list(&lo_destroy_list, ino ? ino : sb->s_root->d_inode,
+			     lrp, 0, recall_cookie);
 
 out_no_fs_call:
 	dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1194,30 +1251,26 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	};
 	struct inode *inode;
 	void *recall_cookie;
-
-	if (clr->clr_file) {
-		inode = igrab(clr->clr_file->fi_inode);
-		if (WARN_ON(!inode))
-			return;
-	} else
-		inode = clr->clr_sb->s_root->d_inode;
+	LIST_HEAD(lo_destroy_list);
 
 	dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
 		clr->clr_client, clr->clr_file);
 
 	if (clr->cb.cbl_recall_type == RETURN_FILE)
 		pnfs_return_file_layouts(clr->clr_client, clr->clr_file, &lr,
-					 NULL);
+					 NULL, &lo_destroy_list);
 	else
 		pnfs_return_client_layouts(clr->clr_client, &lr,
-					   clr->cb.cbl_fsid.major);
+					   clr->cb.cbl_fsid.major,
+					   &lo_destroy_list);
 
 	spin_lock(&layout_lock);
 	recall_cookie = layoutrecall_done(clr);
 	spin_unlock(&layout_lock);
 
-	fs_layout_return(inode, &lr, LR_FLAG_INTERN, recall_cookie);
-	iput(inode);
+	inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode;
+	pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, LR_FLAG_INTERN,
+			     recall_cookie);
 }
 
 /* Return On Close:
@@ -1228,39 +1281,31 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
 {
 	struct nfs4_layout *lo, *nextlp;
-	bool found = false;
+	LIST_HEAD(lo_destroy_list);
 
+	/* TODO: We need to also free layout recalls like pnfs_expire_client */
 	dprintk("%s: fp=%p clp=%p", __func__, fp, clp);
 	spin_lock(&layout_lock);
 	list_for_each_entry_safe (lo, nextlp, &fp->fi_layouts, lo_perfile) {
-		struct nfsd4_pnfs_layoutreturn lr;
-		bool empty;
-
 		/* Check for a match */
 		if (lo->lo_client != clp)
 			continue;
 
 		/* Return the layout */
-		memset(&lr, 0, sizeof(lr));
-		lr.args.lr_return_type = RETURN_FILE;
-		lr.args.lr_seg = lo->lo_seg;
 		list_del_init(&lo->lo_state->ls_perfile);	/* just to be on the safe side */
 		dequeue_layout(lo);
-		destroy_layout(lo); /* do not access lp after this */
-
-		empty = list_empty(&fp->fi_layouts);
-		found = true;
-		dprintk("%s: fp=%p clp=%p: return on close", __func__, fp, clp);
-		fs_layout_return(fp->fi_inode, &lr, LR_FLAG_INTERN,
-				 empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
+		list_add_tail(&lo->lo_perfile, &lo_destroy_list);
 	}
 	spin_unlock(&layout_lock);
-	if (!found)
-		dprintk("%s: no layout found", __func__);
+
+	pnfsd_return_lo_list(&lo_destroy_list, NULL, NULL, LR_FLAG_INTERN, NULL);
 }
 
 void pnfs_expire_client(struct nfs4_client *clp)
 {
+	struct nfs4_layout *lo, *nextlo;
+	LIST_HEAD(lo_destroy_list);
+
 	for (;;) {
 		struct nfs4_layoutrecall *lrp = NULL;
 
@@ -1280,40 +1325,17 @@ void pnfs_expire_client(struct nfs4_client *clp)
 		put_layoutrecall(lrp);
 	}
 
-	for (;;) {
-		struct nfs4_layout *lp = NULL;
-		struct inode *inode = NULL;
-		struct nfsd4_pnfs_layoutreturn lr;
-		bool empty = false;
-
-		spin_lock(&layout_lock);
-		if (!list_empty(&clp->cl_layouts)) {
-			lp = list_entry(clp->cl_layouts.next,
-					struct nfs4_layout, lo_perclnt);
-			inode = igrab(lp->lo_file->fi_inode);
-			memset(&lr, 0, sizeof(lr));
-			lr.args.lr_return_type = RETURN_FILE;
-			lr.args.lr_seg = lp->lo_seg;
-			empty = list_empty(&lp->lo_file->fi_layouts);
-			BUG_ON(lp->lo_client != clp);
-			list_del_init(&lp->lo_state->ls_perfile);	/* just to be on the safe side */
-			dequeue_layout(lp);
-			destroy_layout(lp); /* do not access lp after this */
-		}
-		spin_unlock(&layout_lock);
-		if (!lp)
-			break;
-
-		if (WARN_ON(!inode))
-			break;
-
-		dprintk("%s: inode %lu lp %p clp %p\n", __func__, inode->i_ino,
-			lp, clp);
-
-		fs_layout_return(inode, &lr, LR_FLAG_EXPIRE,
-				 empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
-		iput(inode);
+	spin_lock(&layout_lock);
+	list_for_each_entry_safe (lo, nextlo, &clp->cl_layouts, lo_perclnt) {
+		BUG_ON(lo->lo_client != clp);
+		dequeue_layout(lo);
+		list_add_tail(&lo->lo_perfile, &lo_destroy_list);
+		dprintk("%s: inode %lu lp %p clp %p\n", __func__,
+			lo->lo_file->fi_inode->i_ino, lo, clp);
 	}
+	spin_unlock(&layout_lock);
+
+	pnfsd_return_lo_list(&lo_destroy_list, NULL, NULL, LR_FLAG_EXPIRE, NULL);
 }
 
 struct create_recall_list_arg {
@@ -1516,10 +1538,6 @@ struct create_recall_list_arg {
 
 	INIT_LIST_HEAD(&todolist);
 
-	/* If no cookie provided by FS, return a default one */
-	if (!cbl->cbl_cookie)
-		cbl->cbl_cookie = PNFS_LAST_LAYOUT_NO_RECALLS;
-
 	status = create_layout_recall_list(&todolist, &todo_len, cbl, lrfile);
 	if (list_empty(&todolist)) {
 		status = -ENOENT;
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index 76427d0..ec77175 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -110,8 +110,6 @@ struct nfsd4_pnfs_layoutcommit_res {
 	u64			lc_newsize;	/* response */
 };
 
-#define PNFS_LAST_LAYOUT_NO_RECALLS ((void *)-1) /* used with lr_cookie below */
-
 enum layoutreturn_flags {
 	LR_FLAG_INTERN = 1 << 0,	/* internal return */
 	LR_FLAG_EXPIRE = 1 << 1,	/* return on client expiration */
-- 
1.7.11.7

[PATCH 6/9] pnfsd: LR_FLAG_CL_EMPTY layout return flag

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c             | 17 +++++++++++++----
 include/linux/nfsd/nfsd4_pnfs.h |  1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index d48abba..2d4f23d 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1064,6 +1064,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	u64 ex_fsid = current_fh->fh_export->ex_fsid;
 	void *recall_cookie = NULL;
 	LIST_HEAD(lo_destroy_list);
+	int lr_flags = 0;
 
 	dprintk("NFSD: %s\n", __func__);
 
@@ -1098,9 +1099,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 		layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls,
 							 &lo_destroy_list);
 		put_layout_state(ls);
+		if (!lrp->lrs_present)
+			lr_flags |= LR_FLAG_CL_EMPTY;
 	} else {
 		layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid,
 							   &lo_destroy_list);
+		lr_flags |= LR_FLAG_CL_EMPTY;
 	}
 
 	dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1136,7 +1140,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	nfs4_unlock_state();
 
 	pnfsd_return_lo_list(&lo_destroy_list, ino ? ino : sb->s_root->d_inode,
-			     lrp, 0, recall_cookie);
+			     lrp, lr_flags, recall_cookie);
 
 out_no_fs_call:
 	dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1252,24 +1256,29 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 	struct inode *inode;
 	void *recall_cookie;
 	LIST_HEAD(lo_destroy_list);
+	int lr_flags = LR_FLAG_INTERN;
 
 	dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
 		clr->clr_client, clr->clr_file);
 
-	if (clr->cb.cbl_recall_type == RETURN_FILE)
+	if (clr->cb.cbl_recall_type == RETURN_FILE) {
 		pnfs_return_file_layouts(clr->clr_client, clr->clr_file, &lr,
 					 NULL, &lo_destroy_list);
-	else
+		if (!lr.lrs_present)
+			lr_flags |= LR_FLAG_CL_EMPTY;
+	} else {
 		pnfs_return_client_layouts(clr->clr_client, &lr,
 					   clr->cb.cbl_fsid.major,
 					   &lo_destroy_list);
+		lr_flags |= LR_FLAG_CL_EMPTY;
+	}
 
 	spin_lock(&layout_lock);
 	recall_cookie = layoutrecall_done(clr);
 	spin_unlock(&layout_lock);
 
 	inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode;
-	pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, LR_FLAG_INTERN,
+	pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, lr_flags,
 			     recall_cookie);
 }
 
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index ec77175..bfea9a6 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -113,6 +113,7 @@ struct nfsd4_pnfs_layoutcommit_res {
 enum layoutreturn_flags {
 	LR_FLAG_INTERN = 1 << 0,	/* internal return */
 	LR_FLAG_EXPIRE = 1 << 1,	/* return on client expiration */
+	LR_FLAG_CL_EMPTY = 1 << 2,	/* No more layout for returning client */
 };
 
 struct nfsd4_pnfs_layoutreturn_arg {
-- 
1.7.11.7

[PATCH 7/9] pnfsd: LR_FLAG_EMPTY layout return flag

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c             | 8 +++++++-
 include/linux/nfsd/nfsd4_pnfs.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 2d4f23d..0b8c502 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -921,15 +921,21 @@ struct super_block *
 		struct inode *inode = lo->lo_file->fi_inode;
 		struct nfsd4_pnfs_layoutreturn lr;
 		bool empty;
+		int lr_flags = flags;
 
 		memset(&lr, 0, sizeof(lr));
 		lr.args.lr_return_type = RETURN_FILE;
 		lr.args.lr_seg = lo->lo_seg;
 
+		spin_lock(&layout_lock);
+		if (list_empty(&lo->lo_file->fi_layouts))
+			lr_flags |= LR_FLAG_EMPTY;
+		spin_unlock(&layout_lock);
+
 		list_del(&lo->lo_perfile);
 		empty = list_empty(lo_destroy_list);
 
-		fs_layout_return(inode, &lr, flags, empty ? cb_cookie : NULL);
+		fs_layout_return(inode, &lr, lr_flags, empty ? cb_cookie : NULL);
 
 		destroy_layout(lo); /* this will put the lo_file */
 	}
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index bfea9a6..ecb412e 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -114,6 +114,7 @@ enum layoutreturn_flags {
 	LR_FLAG_INTERN = 1 << 0,	/* internal return */
 	LR_FLAG_EXPIRE = 1 << 1,	/* return on client expiration */
 	LR_FLAG_CL_EMPTY = 1 << 2,	/* No more layout for returning client */
+	LR_FLAG_EMPTY = 1 << 3,		/* No more layout for file */
 };
 
 struct nfsd4_pnfs_layoutreturn_arg {
-- 
1.7.11.7

[PATCH 8/9] SQUASHME: pnfsd: Something very wrong with layout_recall(RETURN_FILE)

[Benny Halevy]

From: Boaz Harrosh <bharrosh@panasas.com>

In patch:
	pnfsd: layout recall layout state

the cl_has_file_layout() is no longer inspecting the layout structures added per file
but is inspecting if file has layout_state.

So it is counting layout_states and not layouts

This is bad because the addition of the layout_states on the file is done before the
call to the filesystem so if the FS does a recall, the nfsd is confused thinking
it already has a layout and issues a recall. Instead of returning -ENOENT, ie list
is empty. The client then truly returns nomaching_layout and when the lo_return(s) are
emulated the system gets stuck is some reference miss-match. (UML so no crash trace)

Now lets say that the state should be set before the call to the FS. Then I don't
see where the state is removed in the case of an ERROR return from FS->layout_get.
Meaning cl_has_file_layout() will always think it has some count.

Also When a layout is returned it is the layout list that is inspected and freed,
so how is the cl_has_file_layout() emptied ?

In any way. I do not agree that it is the state that is needed to be searched
in cl_has_file_layout() but it is layouts that are needed, otherwise the all
layout <---> recall very delicate dance is totally broken.

What was the meaning of the Poet?

I reverted the cl_has_file_layout() to historical processing.

Also cl_has_file_layout() returns true for any layout on a file, but we must
inspect IO_MODE and LSEG for a partial-match, as well.

The below works for me. State also looks good. I can now safely call
cb_recall, from within a layout_get operation.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 0b8c502..3375554 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1192,24 +1192,27 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 }
 
 static bool
-cl_has_file_layout(struct nfs4_client *clp, struct nfs4_file *fp, stateid_t *lsid)
+cl_has_file_layout(struct nfs4_client *clp, struct nfs4_file *fp,
+		   stateid_t *lsid, struct nfsd4_pnfs_cb_layout *cbl)
 {
-	struct nfs4_layout_state *ls;
+	struct nfs4_layout *lo;
+	bool ret = false;
 
 	spin_lock(&layout_lock);
-	list_for_each_entry (ls, &fp->fi_layout_states, ls_perfile)
-		if (same_clid(&ls->ls_stid.sc_stateid.si_opaque.so_clid,
-			      &clp->cl_clientid)) {
+	list_for_each_entry(lo, &fp->fi_layouts, lo_perfile) {
+		if (same_clid(&lo->lo_client->cl_clientid, &clp->cl_clientid) &&
+		    lo_seg_overlapping(&cbl->cbl_seg, &lo->lo_seg) &&
+		    (cbl->cbl_seg.iomode & lo->lo_seg.iomode))
 			goto found;
-		}
-	spin_unlock(&layout_lock);
-	return false;
-
+	}
+	goto unlock;
 found:
-	update_layout_stateid_locked(ls, lsid);
+	/* Im going to send a recall on this latout update state */
+	update_layout_stateid_locked(lo->lo_state, lsid);
+	ret = true;
+unlock:
 	spin_unlock(&layout_lock);
-
-	return true;
+	return ret;
 }
 
 static int
@@ -1241,7 +1244,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
 {
 	switch (cbl->cbl_recall_type) {
 	case RETURN_FILE:
-		return cl_has_file_layout(clp, lrfile, lsid);
+		return cl_has_file_layout(clp, lrfile, lsid, cbl);
 	case RETURN_FSID:
 		return cl_has_fsid_layout(clp, &cbl->cbl_fsid);
 	default:
-- 
1.7.11.7

[PATCH 9/9] SQUASHME: pnfsd: no use for fi_layout_states list anymore

[Benny Halevy]

Signed-off-by: Benny Halevy <bhalevy@tonian.com>
---
 fs/nfsd/nfs4pnfsd.c | 10 ----------
 fs/nfsd/nfs4state.c |  1 -
 fs/nfsd/pnfsd.h     |  1 -
 fs/nfsd/state.h     |  1 -
 4 files changed, 13 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 3375554..4320914 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -157,10 +157,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
 		return new;
 	kref_init(&new->ls_ref);
 	nfsd4_init_stid(&new->ls_stid, clp, NFS4_LAYOUT_STID);
-	INIT_LIST_HEAD(&new->ls_perfile);
-	spin_lock(&layout_lock);
-	list_add(&new->ls_perfile, &fp->fi_layout_states);
-	spin_unlock(&layout_lock);
 	new->ls_roc = false;
 	return new;
 }
@@ -178,11 +174,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
 			container_of(kref, struct nfs4_layout_state, ls_ref);
 
 	nfsd4_unhash_stid(&ls->ls_stid);
-	if (!list_empty(&ls->ls_perfile)) {
-		spin_lock(&layout_lock);
-		list_del(&ls->ls_perfile);
-		spin_unlock(&layout_lock);
-	}
 	kfree(ls);
 }
 
@@ -1310,7 +1301,6 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
 			continue;
 
 		/* Return the layout */
-		list_del_init(&lo->lo_state->ls_perfile);	/* just to be on the safe side */
 		dequeue_layout(lo);
 		list_add_tail(&lo->lo_perfile, &lo_destroy_list);
 	}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0553220..1682413 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2370,7 +2370,6 @@ static void nfsd4_init_file(struct nfs4_file *fp, struct inode *ino,
 	memset(fp->fi_access, 0, sizeof(fp->fi_access));
 #if defined(CONFIG_PNFSD)
 	INIT_LIST_HEAD(&fp->fi_layouts);
-	INIT_LIST_HEAD(&fp->fi_layout_states);
 	fp->fi_fsid.major = current_fh->fh_export->ex_fsid;
 	fp->fi_fsid.minor = 0;
 	fp->fi_fhlen = current_fh->fh_handle.fh_size;
diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h
index 35859ff..fe35466 100644
--- a/fs/nfsd/pnfsd.h
+++ b/fs/nfsd/pnfsd.h
@@ -44,7 +44,6 @@
 struct nfs4_layout_state {
 	struct nfs4_stid	ls_stid; /* must be first field */
 	struct kref		ls_ref;
-	struct list_head	ls_perfile;
 	bool			ls_roc;
 };
 
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 08a31a7..4ca6f2d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -399,7 +399,6 @@ struct nfs4_file {
 	bool			fi_had_conflict;
 #if defined(CONFIG_PNFSD)
 	struct list_head	fi_layouts;
-	struct list_head	fi_layout_states;
 	/* used by layoutget / layoutrecall */
 	struct nfs4_fsid	fi_fsid;
 	u32			fi_fhlen;
-- 
1.7.11.7