[PATCH v2 1/6] NFSv4.1/pnfs: Fixup an lo->plh_block_lgets imbalance in layoutreturn

[PATCH v2 1/6] NFSv4.1/pnfs: Fixup an lo->plh_block_lgets imbalance in layoutreturn

[Trond Myklebust]

Since commit 2d8ae84fbc32, nothing is bumping lo->plh_block_lgets in the
layoutreturn path, so it should not be touched in nfs4_layoutreturn_release
either.

Fixes: 2d8ae84fbc32 ("NFSv4.1/pnfs: Remove redundant lo->plh_block_lgets...")
Cc: stable@vger.kernel.org # 4.3+
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index adae525edec4..306a8a0cf5bd 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8077,7 +8077,6 @@ static void nfs4_layoutreturn_release(void *calldata)
 		pnfs_set_layout_stateid(lo, &lrp->res.stateid, true);
 	pnfs_mark_matching_lsegs_invalid(lo, &freeme, &lrp->args.range);
 	pnfs_clear_layoutreturn_waitbit(lo);
-	lo->plh_block_lgets--;
 	spin_unlock(&lo->plh_inode->i_lock);
 	pnfs_free_lseg_list(&freeme);
 	pnfs_put_layout_hdr(lrp->args.layout);
-- 
2.5.0

[PATCH v2 2/6] pNFS: Ensure nfs4_layoutget_prepare returns the correct error

[Trond Myklebust]

If we're unable to perform the layoutget due to an invalid open stateid
or a bulk recall, ensure that we return the error so that the caller
can decide on an appropriate action.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 306a8a0cf5bd..e643fe08e015 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7776,6 +7776,7 @@ nfs4_layoutget_prepare(struct rpc_task *task, void *calldata)
 	struct nfs4_layoutget *lgp = calldata;
 	struct nfs_server *server = NFS_SERVER(lgp->args.inode);
 	struct nfs4_session *session = nfs4_get_session(server);
+	int ret;
 
 	dprintk("--> %s\n", __func__);
 	/* Note the is a race here, where a CB_LAYOUTRECALL can come in
@@ -7786,12 +7787,12 @@ nfs4_layoutget_prepare(struct rpc_task *task, void *calldata)
 	if (nfs41_setup_sequence(session, &lgp->args.seq_args,
 				&lgp->res.seq_res, task))
 		return;
-	if (pnfs_choose_layoutget_stateid(&lgp->args.stateid,
+	ret = pnfs_choose_layoutget_stateid(&lgp->args.stateid,
 					  NFS_I(lgp->args.inode)->layout,
 					  &lgp->args.range,
-					  lgp->args.ctx->state)) {
-		rpc_exit(task, NFS4_OK);
-	}
+					  lgp->args.ctx->state);
+	if (ret < 0)
+		rpc_exit(task, ret);
 }
 
 static void nfs4_layoutget_done(struct rpc_task *task, void *calldata)
-- 
2.5.0

[PATCH v2 3/6] NFSv4.1/pNFS: Add a helper to mark the layout as returned

[Trond Myklebust]

This ensures that we don't reuse the stateid if a layout return or
implied layout return means that we've returned all layout segments

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c |  1 +
 fs/nfs/nfs4proc.c      |  3 ++-
 fs/nfs/pnfs.c          |  1 +
 fs/nfs/pnfs.h          | 13 +++++++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 807eb6ef4f91..716cbff24450 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -192,6 +192,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 		NFS_SERVER(ino)->pnfs_curr_ld->return_range(lo,
 			&args->cbl_range);
 	}
+	pnfs_mark_layout_returned_if_empty(lo);
 unlock:
 	spin_unlock(&ino->i_lock);
 	pnfs_free_lseg_list(&free_me_list);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e643fe08e015..10bc91ed6c5a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8074,9 +8074,10 @@ static void nfs4_layoutreturn_release(void *calldata)
 
 	dprintk("--> %s\n", __func__);
 	spin_lock(&lo->plh_inode->i_lock);
+	pnfs_mark_matching_lsegs_invalid(lo, &freeme, &lrp->args.range);
+	pnfs_mark_layout_returned_if_empty(lo);
 	if (lrp->res.lrs_present)
 		pnfs_set_layout_stateid(lo, &lrp->res.stateid, true);
-	pnfs_mark_matching_lsegs_invalid(lo, &freeme, &lrp->args.range);
 	pnfs_clear_layoutreturn_waitbit(lo);
 	spin_unlock(&lo->plh_inode->i_lock);
 	pnfs_free_lseg_list(&freeme);
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 113c3b327e24..11bdc59b7c5a 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1139,6 +1139,7 @@ void pnfs_roc_set_barrier(struct inode *ino, u32 barrier)
 
 	spin_lock(&ino->i_lock);
 	lo = NFS_I(ino)->layout;
+	pnfs_mark_layout_returned_if_empty(lo);
 	if (pnfs_seqid_is_newer(barrier, lo->plh_barrier))
 		lo->plh_barrier = barrier;
 	spin_unlock(&ino->i_lock);
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 6916ff4e86f9..4e4ac660adbd 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -535,6 +535,19 @@ pnfs_calc_offset_length(u64 offset, u64 end)
 	return 1 + end - offset;
 }
 
+/**
+ * pnfs_mark_layout_returned_if_empty - marks the layout as returned
+ * @lo: layout header
+ *
+ * Note: Caller must hold inode->i_lock
+ */
+static inline void
+pnfs_mark_layout_returned_if_empty(struct pnfs_layout_hdr *lo)
+{
+	if (list_empty(&lo->plh_segs))
+		set_bit(NFS_LAYOUT_INVALID_STID, &lo->plh_flags);
+}
+
 extern unsigned int layoutstats_timer;
 
 #ifdef NFS_DEBUG
-- 
2.5.0

[PATCH v2 4/6] pNFS: If we have to delay the layout callback, mark the layout for return

[Trond Myklebust]

If the client needs to delay the layout callback, then speed up the recall
process by marking the remaining layout segments to be actively returned
by the client.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 14 ++++++++++++--
 fs/nfs/pnfs.c          |  4 +++-
 fs/nfs/pnfs.h          |  3 +++
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 716cbff24450..34852ece4057 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -181,9 +181,19 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 	pnfs_layoutcommit_inode(ino, false);
 
 	spin_lock(&ino->i_lock);
-	if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags) ||
-	    pnfs_mark_matching_lsegs_invalid(lo, &free_me_list,
+	/*
+	 * Enforce RFC5661 Section 12.5.5.2.1.5 (Bulk Recall and Return)
+	 */
+	if (test_bit(NFS_LAYOUT_BULK_RECALL, &lo->plh_flags)) {
+		rv = NFS4ERR_DELAY;
+		goto unlock;
+	}
+
+	if (pnfs_mark_matching_lsegs_invalid(lo, &free_me_list,
 					&args->cbl_range)) {
+		pnfs_mark_matching_lsegs_return(lo,
+				&free_me_list,
+				&args->cbl_range);
 		rv = NFS4ERR_DELAY;
 		goto unlock;
 	}
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 11bdc59b7c5a..667d986ac020 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1734,7 +1734,7 @@ out_forget_reply:
 	goto out;
 }
 
-static void
+void
 pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 				struct list_head *tmp_list,
 				struct pnfs_layout_range *return_range)
@@ -1746,6 +1746,8 @@ pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
 	if (list_empty(&lo->plh_segs))
 		return;
 
+	assert_spin_locked(&lo->plh_inode->i_lock);
+
 	list_for_each_entry_safe(lseg, next, &lo->plh_segs, pls_list)
 		if (should_free_lseg(&lseg->pls_range, return_range)) {
 			dprintk("%s: marking lseg %p iomode %d "
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 4e4ac660adbd..6f751a960336 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -265,6 +265,9 @@ int pnfs_choose_layoutget_stateid(nfs4_stateid *dst,
 int pnfs_mark_matching_lsegs_invalid(struct pnfs_layout_hdr *lo,
 				struct list_head *tmp_list,
 				struct pnfs_layout_range *recall_range);
+void pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
+				struct list_head *tmp_list,
+				struct pnfs_layout_range *recall_range);
 bool pnfs_roc(struct inode *ino);
 void pnfs_roc_release(struct inode *ino);
 void pnfs_roc_set_barrier(struct inode *ino, u32 barrier);
-- 
2.5.0

[PATCH v2 5/6] NFSv4.1/pNFS: Ensure we enforce RFC5661 Section 12.5.5.2.1

[Trond Myklebust]

The RFC requires us to check if the server is recalling a stateid that we
haven't yet received. If so, tell it to wait.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 34852ece4057..1b24ad07d4f5 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -160,6 +160,22 @@ static struct pnfs_layout_hdr * get_layout_by_fh(struct nfs_client *clp,
 	return lo;
 }
 
+/*
+ * Enforce RFC5661 section 12.5.5.2.1. (Layout Recall and Return Sequencing)
+ */
+static bool pnfs_check_stateid_sequence(struct pnfs_layout_hdr *lo,
+					const nfs4_stateid *new)
+{
+	u32 oldseq, newseq;
+
+	oldseq = be32_to_cpu(lo->plh_stateid.seqid);
+	newseq = be32_to_cpu(new->seqid);
+
+	if (newseq > oldseq + 1)
+		return false;
+	return true;
+}
+
 static u32 initiate_file_draining(struct nfs_client *clp,
 				  struct cb_layoutrecallargs *args)
 {
@@ -175,6 +191,10 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 	ino = lo->plh_inode;
 
 	spin_lock(&ino->i_lock);
+	if (!pnfs_check_stateid_sequence(lo, &args->cbl_stateid)) {
+		rv = NFS4ERR_DELAY;
+		goto unlock;
+	}
 	pnfs_set_layout_stateid(lo, &args->cbl_stateid, true);
 	spin_unlock(&ino->i_lock);
 
-- 
2.5.0

[PATCH v2 6/6] NFSv4.1/pNFS: Don't return NFS4ERR_DELAY unnecessarily in CB_LAYOUTRECALL

[Trond Myklebust]

If the client is promising to return the layout ASAP, then there is no
need to return DELAY and have the server retry. Instead default to the
normal procedure described in RFC5661.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 1b24ad07d4f5..724a9b756ab0 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -214,7 +214,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 		pnfs_mark_matching_lsegs_return(lo,
 				&free_me_list,
 				&args->cbl_range);
-		rv = NFS4ERR_DELAY;
+		rv = NFS4_OK;
 		goto unlock;
 	}
 
-- 
2.5.0

Re: [PATCH v2 6/6] NFSv4.1/pNFS: Don't return NFS4ERR_DELAY unnecessarily in CB_LAYOUTRECALL

[Peng Tao]

On Tue, Dec 29, 2015 at 2:05 AM, Trond Myklebust
<trond.myklebust@primarydata.com> wrote:
> If the client is promising to return the layout ASAP, then there is no
> need to return DELAY and have the server retry. Instead default to the
> normal procedure described in RFC5661.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  fs/nfs/callback_proc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 1b24ad07d4f5..724a9b756ab0 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -214,7 +214,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
>                 pnfs_mark_matching_lsegs_return(lo,
>                                 &free_me_list,
>                                 &args->cbl_range);
> -               rv = NFS4ERR_DELAY;
> +               rv = NFS4_OK;
It is possible that we've cleared all outstanding layout segments of
the file and returns NFS4_OK to CB_LAYOUTRECALL. Then server would be
expecting LAYOUTRETURN while client will not send one. IMO we'd better
re-check list_empty(&lo->plh_segs) to decide what to return here.

Cheers,
Tao

Re: [PATCH v2 6/6] NFSv4.1/pNFS: Don't return NFS4ERR_DELAY unnecessarily in CB_LAYOUTRECALL

[Trond Myklebust]

Hi Tao,

On Mon, Jan 4, 2016 at 10:13 AM, Peng Tao <bergwolf@primarydata.com> wrote:
> On Tue, Dec 29, 2015 at 2:05 AM, Trond Myklebust
> <trond.myklebust@primarydata.com> wrote:
>> If the client is promising to return the layout ASAP, then there is no
>> need to return DELAY and have the server retry. Instead default to the
>> normal procedure described in RFC5661.
>>
>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>> ---
>>  fs/nfs/callback_proc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index 1b24ad07d4f5..724a9b756ab0 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -214,7 +214,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
>>                 pnfs_mark_matching_lsegs_return(lo,
>>                                 &free_me_list,
>>                                 &args->cbl_range);
>> -               rv = NFS4ERR_DELAY;
>> +               rv = NFS4_OK;
> It is possible that we've cleared all outstanding layout segments of
> the file and returns NFS4_OK to CB_LAYOUTRECALL. Then server would be
> expecting LAYOUTRETURN while client will not send one. IMO we'd better
> re-check list_empty(&lo->plh_segs) to decide what to return here.
>

Thanks! I'll fix this up in v3...

Cheers
  Trond