|
|
642afd4 |
From 675f11241a9f5b434effc7aee9eb84bf3d17d685 Mon Sep 17 00:00:00 2001
|
|
|
642afd4 |
From: Trond Myklebust <trond.myklebust@hammerspace.com>
|
|
|
642afd4 |
Date: Wed, 20 Jun 2018 17:53:34 -0400
|
|
|
642afd4 |
Subject: [PATCH] NFSv4.1: Avoid false retries when RPC calls are interrupted
|
|
|
642afd4 |
|
|
|
642afd4 |
A 'false retry' in NFSv4.1 occurs when the client attempts to transmit a
|
|
|
642afd4 |
new RPC call using a slot+sequence number combination that references an
|
|
|
642afd4 |
already cached one. Currently, the Linux NFS client will do this if a
|
|
|
642afd4 |
user process interrupts an RPC call that is in progress.
|
|
|
642afd4 |
The problem with doing so is that we defeat the main mechanism used by
|
|
|
642afd4 |
the server to differentiate between a new call and a replayed one. Even
|
|
|
642afd4 |
if the server is able to perfectly cache the arguments of the old call,
|
|
|
642afd4 |
it cannot know if the client intended to replay or send a new call.
|
|
|
642afd4 |
|
|
|
642afd4 |
The obvious fix is to bump the sequence number pre-emptively if an
|
|
|
642afd4 |
RPC call is interrupted, but in order to deal with the corner cases
|
|
|
642afd4 |
where the interrupted call is not actually received and processed by
|
|
|
642afd4 |
the server, we need to interpret the error NFS4ERR_SEQ_MISORDERED
|
|
|
642afd4 |
as a sign that we need to either wait or locate a correct sequence
|
|
|
642afd4 |
number that lies between the value we sent, and the last value that
|
|
|
642afd4 |
was acked by a SEQUENCE call on that slot.
|
|
|
642afd4 |
|
|
|
642afd4 |
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
|
|
|
642afd4 |
---
|
|
|
642afd4 |
fs/nfs/nfs4proc.c | 105 ++++++++++++++++++++-----------------------
|
|
|
642afd4 |
fs/nfs/nfs4session.c | 5 ++-
|
|
|
642afd4 |
fs/nfs/nfs4session.h | 5 ++-
|
|
|
642afd4 |
3 files changed, 55 insertions(+), 60 deletions(-)
|
|
|
642afd4 |
|
|
|
642afd4 |
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
|
|
|
642afd4 |
index 64ac80ec6b7b..3a6a9c9ee369 100644
|
|
|
642afd4 |
--- a/fs/nfs/nfs4proc.c
|
|
|
642afd4 |
+++ b/fs/nfs/nfs4proc.c
|
|
|
642afd4 |
@@ -730,13 +730,25 @@ static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
|
|
|
642afd4 |
res->sr_slot = NULL;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
|
|
|
642afd4 |
+static void nfs4_slot_sequence_record_sent(struct nfs4_slot *slot,
|
|
|
642afd4 |
+ u32 seqnr)
|
|
|
642afd4 |
+{
|
|
|
642afd4 |
+ if ((s32)(seqnr - slot->seq_nr_highest_sent) > 0)
|
|
|
642afd4 |
+ slot->seq_nr_highest_sent = seqnr;
|
|
|
642afd4 |
+}
|
|
|
642afd4 |
+static void nfs4_slot_sequence_acked(struct nfs4_slot *slot,
|
|
|
642afd4 |
+ u32 seqnr)
|
|
|
642afd4 |
+{
|
|
|
642afd4 |
+ slot->seq_nr_highest_sent = seqnr;
|
|
|
642afd4 |
+ slot->seq_nr_last_acked = seqnr;
|
|
|
642afd4 |
+}
|
|
|
642afd4 |
+
|
|
|
642afd4 |
static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
struct nfs4_sequence_res *res)
|
|
|
642afd4 |
{
|
|
|
642afd4 |
struct nfs4_session *session;
|
|
|
642afd4 |
struct nfs4_slot *slot = res->sr_slot;
|
|
|
642afd4 |
struct nfs_client *clp;
|
|
|
642afd4 |
- bool interrupted = false;
|
|
|
642afd4 |
int ret = 1;
|
|
|
642afd4 |
|
|
|
642afd4 |
if (slot == NULL)
|
|
|
642afd4 |
@@ -747,16 +759,12 @@ static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
|
|
|
642afd4 |
session = slot->table->session;
|
|
|
642afd4 |
|
|
|
642afd4 |
- if (slot->interrupted) {
|
|
|
642afd4 |
- if (res->sr_status != -NFS4ERR_DELAY)
|
|
|
642afd4 |
- slot->interrupted = 0;
|
|
|
642afd4 |
- interrupted = true;
|
|
|
642afd4 |
- }
|
|
|
642afd4 |
-
|
|
|
642afd4 |
trace_nfs4_sequence_done(session, res);
|
|
|
642afd4 |
/* Check the SEQUENCE operation status */
|
|
|
642afd4 |
switch (res->sr_status) {
|
|
|
642afd4 |
case 0:
|
|
|
642afd4 |
+ /* Mark this sequence number as having been acked */
|
|
|
642afd4 |
+ nfs4_slot_sequence_acked(slot, slot->seq_nr);
|
|
|
642afd4 |
/* Update the slot's sequence and clientid lease timer */
|
|
|
642afd4 |
slot->seq_done = 1;
|
|
|
642afd4 |
clp = session->clp;
|
|
|
642afd4 |
@@ -771,9 +779,9 @@ static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
* sr_status remains 1 if an RPC level error occurred.
|
|
|
642afd4 |
* The server may or may not have processed the sequence
|
|
|
642afd4 |
* operation..
|
|
|
642afd4 |
- * Mark the slot as having hosted an interrupted RPC call.
|
|
|
642afd4 |
*/
|
|
|
642afd4 |
- slot->interrupted = 1;
|
|
|
642afd4 |
+ nfs4_slot_sequence_record_sent(slot, slot->seq_nr);
|
|
|
642afd4 |
+ slot->seq_done = 1;
|
|
|
642afd4 |
goto out;
|
|
|
642afd4 |
case -NFS4ERR_DELAY:
|
|
|
642afd4 |
/* The server detected a resend of the RPC call and
|
|
|
642afd4 |
@@ -784,6 +792,7 @@ static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
__func__,
|
|
|
642afd4 |
slot->slot_nr,
|
|
|
642afd4 |
slot->seq_nr);
|
|
|
642afd4 |
+ nfs4_slot_sequence_acked(slot, slot->seq_nr);
|
|
|
642afd4 |
goto out_retry;
|
|
|
642afd4 |
case -NFS4ERR_RETRY_UNCACHED_REP:
|
|
|
642afd4 |
case -NFS4ERR_SEQ_FALSE_RETRY:
|
|
|
642afd4 |
@@ -791,6 +800,7 @@ static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
* The server thinks we tried to replay a request.
|
|
|
642afd4 |
* Retry the call after bumping the sequence ID.
|
|
|
642afd4 |
*/
|
|
|
642afd4 |
+ nfs4_slot_sequence_acked(slot, slot->seq_nr);
|
|
|
642afd4 |
goto retry_new_seq;
|
|
|
642afd4 |
case -NFS4ERR_BADSLOT:
|
|
|
642afd4 |
/*
|
|
|
642afd4 |
@@ -801,21 +811,28 @@ static int nfs41_sequence_process(struct rpc_task *task,
|
|
|
642afd4 |
goto session_recover;
|
|
|
642afd4 |
goto retry_nowait;
|
|
|
642afd4 |
case -NFS4ERR_SEQ_MISORDERED:
|
|
|
642afd4 |
+ nfs4_slot_sequence_record_sent(slot, slot->seq_nr);
|
|
|
642afd4 |
/*
|
|
|
642afd4 |
- * Was the last operation on this sequence interrupted?
|
|
|
642afd4 |
- * If so, retry after bumping the sequence number.
|
|
|
642afd4 |
+ * Were one or more calls using this slot interrupted?
|
|
|
642afd4 |
+ * If the server never received the request, then our
|
|
|
642afd4 |
+ * transmitted slot sequence number may be too high.
|
|
|
642afd4 |
*/
|
|
|
642afd4 |
- if (interrupted)
|
|
|
642afd4 |
- goto retry_new_seq;
|
|
|
642afd4 |
- /*
|
|
|
642afd4 |
- * Could this slot have been previously retired?
|
|
|
642afd4 |
- * If so, then the server may be expecting seq_nr = 1!
|
|
|
642afd4 |
- */
|
|
|
642afd4 |
- if (slot->seq_nr != 1) {
|
|
|
642afd4 |
- slot->seq_nr = 1;
|
|
|
642afd4 |
+ if ((s32)(slot->seq_nr - slot->seq_nr_last_acked) > 1) {
|
|
|
642afd4 |
+ slot->seq_nr--;
|
|
|
642afd4 |
goto retry_nowait;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
- goto session_recover;
|
|
|
642afd4 |
+ /*
|
|
|
642afd4 |
+ * RFC5661:
|
|
|
642afd4 |
+ * A retry might be sent while the original request is
|
|
|
642afd4 |
+ * still in progress on the replier. The replier SHOULD
|
|
|
642afd4 |
+ * deal with the issue by returning NFS4ERR_DELAY as the
|
|
|
642afd4 |
+ * reply to SEQUENCE or CB_SEQUENCE operation, but
|
|
|
642afd4 |
+ * implementations MAY return NFS4ERR_SEQ_MISORDERED.
|
|
|
642afd4 |
+ *
|
|
|
642afd4 |
+ * Restart the search after a delay.
|
|
|
642afd4 |
+ */
|
|
|
642afd4 |
+ slot->seq_nr = slot->seq_nr_highest_sent;
|
|
|
642afd4 |
+ goto out_retry;
|
|
|
642afd4 |
default:
|
|
|
642afd4 |
/* Just update the slot sequence no. */
|
|
|
642afd4 |
slot->seq_done = 1;
|
|
|
642afd4 |
@@ -906,17 +923,6 @@ static const struct rpc_call_ops nfs41_call_sync_ops = {
|
|
|
642afd4 |
.rpc_call_done = nfs41_call_sync_done,
|
|
|
642afd4 |
};
|
|
|
642afd4 |
|
|
|
642afd4 |
-static void
|
|
|
642afd4 |
-nfs4_sequence_process_interrupted(struct nfs_client *client,
|
|
|
642afd4 |
- struct nfs4_slot *slot, const struct cred *cred)
|
|
|
642afd4 |
-{
|
|
|
642afd4 |
- struct rpc_task *task;
|
|
|
642afd4 |
-
|
|
|
642afd4 |
- task = _nfs41_proc_sequence(client, cred, slot, true);
|
|
|
642afd4 |
- if (!IS_ERR(task))
|
|
|
642afd4 |
- rpc_put_task_async(task);
|
|
|
642afd4 |
-}
|
|
|
642afd4 |
-
|
|
|
642afd4 |
#else /* !CONFIG_NFS_V4_1 */
|
|
|
642afd4 |
|
|
|
642afd4 |
static int nfs4_sequence_process(struct rpc_task *task, struct nfs4_sequence_res *res)
|
|
|
642afd4 |
@@ -937,14 +943,6 @@ int nfs4_sequence_done(struct rpc_task *task,
|
|
|
642afd4 |
}
|
|
|
642afd4 |
EXPORT_SYMBOL_GPL(nfs4_sequence_done);
|
|
|
642afd4 |
|
|
|
642afd4 |
-static void
|
|
|
642afd4 |
-nfs4_sequence_process_interrupted(struct nfs_client *client,
|
|
|
642afd4 |
- struct nfs4_slot *slot, const struct cred *cred)
|
|
|
642afd4 |
-{
|
|
|
642afd4 |
- WARN_ON_ONCE(1);
|
|
|
642afd4 |
- slot->interrupted = 0;
|
|
|
642afd4 |
-}
|
|
|
642afd4 |
-
|
|
|
642afd4 |
#endif /* !CONFIG_NFS_V4_1 */
|
|
|
642afd4 |
|
|
|
642afd4 |
static void nfs41_sequence_res_init(struct nfs4_sequence_res *res)
|
|
|
642afd4 |
@@ -985,26 +983,19 @@ int nfs4_setup_sequence(struct nfs_client *client,
|
|
|
642afd4 |
task->tk_timeout = 0;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
|
|
|
642afd4 |
- for (;;) {
|
|
|
642afd4 |
- spin_lock(&tbl->slot_tbl_lock);
|
|
|
642afd4 |
- /* The state manager will wait until the slot table is empty */
|
|
|
642afd4 |
- if (nfs4_slot_tbl_draining(tbl) && !args->sa_privileged)
|
|
|
642afd4 |
- goto out_sleep;
|
|
|
642afd4 |
-
|
|
|
642afd4 |
- slot = nfs4_alloc_slot(tbl);
|
|
|
642afd4 |
- if (IS_ERR(slot)) {
|
|
|
642afd4 |
- /* Try again in 1/4 second */
|
|
|
642afd4 |
- if (slot == ERR_PTR(-ENOMEM))
|
|
|
642afd4 |
- task->tk_timeout = HZ >> 2;
|
|
|
642afd4 |
- goto out_sleep;
|
|
|
642afd4 |
- }
|
|
|
642afd4 |
- spin_unlock(&tbl->slot_tbl_lock);
|
|
|
642afd4 |
+ spin_lock(&tbl->slot_tbl_lock);
|
|
|
642afd4 |
+ /* The state manager will wait until the slot table is empty */
|
|
|
642afd4 |
+ if (nfs4_slot_tbl_draining(tbl) && !args->sa_privileged)
|
|
|
642afd4 |
+ goto out_sleep;
|
|
|
642afd4 |
|
|
|
642afd4 |
- if (likely(!slot->interrupted))
|
|
|
642afd4 |
- break;
|
|
|
642afd4 |
- nfs4_sequence_process_interrupted(client,
|
|
|
642afd4 |
- slot, task->tk_msg.rpc_cred);
|
|
|
642afd4 |
+ slot = nfs4_alloc_slot(tbl);
|
|
|
642afd4 |
+ if (IS_ERR(slot)) {
|
|
|
642afd4 |
+ /* Try again in 1/4 second */
|
|
|
642afd4 |
+ if (slot == ERR_PTR(-ENOMEM))
|
|
|
642afd4 |
+ task->tk_timeout = HZ >> 2;
|
|
|
642afd4 |
+ goto out_sleep;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
+ spin_unlock(&tbl->slot_tbl_lock);
|
|
|
642afd4 |
|
|
|
642afd4 |
nfs4_sequence_attach_slot(args, res, slot);
|
|
|
642afd4 |
|
|
|
642afd4 |
diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
|
|
|
642afd4 |
index a5489d70a724..39962c19744f 100644
|
|
|
642afd4 |
--- a/fs/nfs/nfs4session.c
|
|
|
642afd4 |
+++ b/fs/nfs/nfs4session.c
|
|
|
642afd4 |
@@ -110,6 +110,8 @@ static struct nfs4_slot *nfs4_new_slot(struct nfs4_slot_table *tbl,
|
|
|
642afd4 |
slot->table = tbl;
|
|
|
642afd4 |
slot->slot_nr = slotid;
|
|
|
642afd4 |
slot->seq_nr = seq_init;
|
|
|
642afd4 |
+ slot->seq_nr_highest_sent = seq_init;
|
|
|
642afd4 |
+ slot->seq_nr_last_acked = seq_init - 1;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
return slot;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
@@ -276,7 +278,8 @@ static void nfs4_reset_slot_table(struct nfs4_slot_table *tbl,
|
|
|
642afd4 |
p = &tbl->slots;
|
|
|
642afd4 |
while (*p) {
|
|
|
642afd4 |
(*p)->seq_nr = ivalue;
|
|
|
642afd4 |
- (*p)->interrupted = 0;
|
|
|
642afd4 |
+ (*p)->seq_nr_highest_sent = ivalue;
|
|
|
642afd4 |
+ (*p)->seq_nr_last_acked = ivalue - 1;
|
|
|
642afd4 |
p = &(*p)->next;
|
|
|
642afd4 |
}
|
|
|
642afd4 |
tbl->highest_used_slotid = NFS4_NO_SLOT;
|
|
|
642afd4 |
diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
|
|
|
642afd4 |
index 3c550f297561..230509b77121 100644
|
|
|
642afd4 |
--- a/fs/nfs/nfs4session.h
|
|
|
642afd4 |
+++ b/fs/nfs/nfs4session.h
|
|
|
642afd4 |
@@ -23,8 +23,9 @@ struct nfs4_slot {
|
|
|
642afd4 |
unsigned long generation;
|
|
|
642afd4 |
u32 slot_nr;
|
|
|
642afd4 |
u32 seq_nr;
|
|
|
642afd4 |
- unsigned int interrupted : 1,
|
|
|
642afd4 |
- privileged : 1,
|
|
|
642afd4 |
+ u32 seq_nr_last_acked;
|
|
|
642afd4 |
+ u32 seq_nr_highest_sent;
|
|
|
642afd4 |
+ unsigned int privileged : 1,
|
|
|
642afd4 |
seq_done : 1;
|
|
|
642afd4 |
};
|
|
|
642afd4 |
|
|
|
642afd4 |
--
|
|
|
642afd4 |
2.20.1
|
|
|
642afd4 |
|