2015-12-28 18:05:56

by Trond Myklebust

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

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: [email protected] # 4.3+
Signed-off-by: Trond Myklebust <[email protected]>
---
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



2015-12-28 18:05:57

by Trond Myklebust

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

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 <[email protected]>
---
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


2015-12-28 18:05:57

by Trond Myklebust

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

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 <[email protected]>
---
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


2015-12-28 18:06:00

by Trond Myklebust

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

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 <[email protected]>
---
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


2015-12-28 18:05:58

by Trond Myklebust

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

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 <[email protected]>
---
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


2015-12-28 18:05:59

by Trond Myklebust

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

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 <[email protected]>
---
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


2016-01-04 15:14:19

by Peng Tao

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

On Tue, Dec 29, 2015 at 2:05 AM, Trond Myklebust
<[email protected]> 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 <[email protected]>
> ---
> 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

2016-01-04 16:37:15

by Trond Myklebust

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

Hi Tao,

On Mon, Jan 4, 2016 at 10:13 AM, Peng Tao <[email protected]> wrote:
> On Tue, Dec 29, 2015 at 2:05 AM, Trond Myklebust
> <[email protected]> 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 <[email protected]>
>> ---
>> 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