All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/10] Fix OPEN/CLOSE races
@ 2017-11-06 15:27 Trond Myklebust
  2017-11-06 15:27 ` [PATCH v7 01/10] NFSv4: Fix OPEN / CLOSE race Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

v2:
- Fix a sleep-while-atomic issue
- Clean up.    
- Add a tracepoint to help document wait incidents.
v3:
- Fix a state update issue
v4:
- Fix a race due to setting the state->flags before waiting on another
  compound (can cause issues if the other compound has an OPEN_DOWNGRADE).
- Fix stateid seqid wraparound.
v5:
- Fix race with wakeup after clearing NFS_STATE_CHANGE_WAIT
- Fix CLOSE, DELEGRETURN and LAYOUTRETURN issues with
  NFS4ERR_OLD_STATEID, which were causing stateid leakage
- cleanups
- Fix a typo in nfs_rename...
v6:
- Fix a compile issue when CONFIG_NFS_V4_1=n
v7:
- Fix a typo in nfs4_refresh_open_stateid (thanks Andrew Elble)
- Minor cleanups in nfs4_close_done, nfs4_delegreturn_done,
  nfs4_layoutreturn_done.

Trond Myklebust (10):
  NFSv4: Fix OPEN / CLOSE race
  NFSv4: Add a tracepoint to document open stateid updates
  NFSv4: Fix open create exclusive when the server reboots
  NFS: Fix a typo in nfs_rename()
  NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.
  NFSv4: Don't try to CLOSE if the stateid 'other' field has changed
  pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close
  NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn
  NFSv4: cleanup nfs4_close_done
  NFSv4: Clean up nfs4_delegreturn_done

 fs/nfs/delegation.c |  27 +++++
 fs/nfs/delegation.h |   1 +
 fs/nfs/dir.c        |   2 +-
 fs/nfs/nfs4_fs.h    |   7 ++
 fs/nfs/nfs4proc.c   | 292 +++++++++++++++++++++++++++++++++++++---------------
 fs/nfs/nfs4state.c  |  26 ++++-
 fs/nfs/nfs4trace.h  |   2 +
 fs/nfs/pnfs.c       |  18 ++++
 fs/nfs/pnfs.h       |   5 +
 9 files changed, 293 insertions(+), 87 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v7 01/10] NFSv4: Fix OPEN / CLOSE race
  2017-11-06 15:27 [PATCH v7 00/10] Fix OPEN/CLOSE races Trond Myklebust
@ 2017-11-06 15:27 ` Trond Myklebust
  2017-11-06 15:27   ` [PATCH v7 02/10] NFSv4: Add a tracepoint to document open stateid updates Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

Ben Coddington has noted the following race between OPEN and CLOSE
on a single client.

Process 1		Process 2		Server
=========		=========		======

1)  OPEN file
2)			OPEN file
3)						Process OPEN (1) seqid=1
4)						Process OPEN (2) seqid=2
5)						Reply OPEN (2)
6)			Receive reply (2)
7)			new stateid, seqid=2

8)			CLOSE file, using
			stateid w/ seqid=2
9)						Reply OPEN (1)
10(						Process CLOSE (8)
11)						Reply CLOSE (8)
12)						Forget stateid
						file closed

13)			Receive reply (7)
14)			Forget stateid
			file closed.

15) Receive reply (1).
16) New stateid seqid=1
    is really the same
    stateid that was
    closed.

IOW: the reply to the first OPEN is delayed. Since "Process 2" does
not wait before closing the file, and it does not cache the closed
stateid, then when the delayed reply is finally received, it is treated
as setting up a new stateid by the client.

The fix is to ensure that the client processes the OPEN and CLOSE calls
in the same order in which the server processed them.

This commit ensures that we examine the seqid of the stateid
returned by OPEN. If it is a new stateid, we assume the seqid
must be equal to the value 1, and that each state transition
increments the seqid value by 1 (See RFC7530, Section 9.1.4.2,
and RFC5661, Section 8.2.2).

If the tracker sees that an OPEN returns with a seqid that is greater
than the cached seqid + 1, then it bumps a flag to ensure that the
caller waits for the RPCs carrying the missing seqids to complete.

Note that there can still be pathologies where the server crashes before
it can even send us the missing seqids. Since the OPEN call is still
holding a slot when it waits here, that could cause the recovery to
stall forever. To avoid that, we time out after a 5 second wait.

Reported-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4_fs.h   |   3 ++
 fs/nfs/nfs4proc.c  | 153 +++++++++++++++++++++++++++++++++++++++++------------
 fs/nfs/nfs4state.c |   1 +
 3 files changed, 122 insertions(+), 35 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index a73144b3cb8c..b07124ae5be1 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -162,6 +162,7 @@ enum {
 	NFS_STATE_POSIX_LOCKS,		/* Posix locks are supported */
 	NFS_STATE_RECOVERY_FAILED,	/* OPEN stateid state recovery failed */
 	NFS_STATE_MAY_NOTIFY_LOCK,	/* server may CB_NOTIFY_LOCK */
+	NFS_STATE_CHANGE_WAIT,		/* A state changing operation is outstanding */
 };
 
 struct nfs4_state {
@@ -185,6 +186,8 @@ struct nfs4_state {
 	unsigned int n_rdwr;		/* Number of read/write references */
 	fmode_t state;			/* State on the server (R,W, or RW) */
 	atomic_t count;
+
+	wait_queue_head_t waitq;
 };
 
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 96b2077e691d..6e3cfa00b4ea 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1378,6 +1378,25 @@ static bool nfs_open_stateid_recover_openmode(struct nfs4_state *state)
 }
 #endif /* CONFIG_NFS_V4_1 */
 
+static void nfs_state_log_update_open_stateid(struct nfs4_state *state)
+{
+	if (test_and_clear_bit(NFS_STATE_CHANGE_WAIT, &state->flags))
+		wake_up_all(&state->waitq);
+}
+
+static void nfs_state_log_out_of_order_open_stateid(struct nfs4_state *state,
+		const nfs4_stateid *stateid)
+{
+	u32 state_seqid = be32_to_cpu(state->open_stateid.seqid);
+	u32 stateid_seqid = be32_to_cpu(stateid->seqid);
+
+	if (stateid_seqid == state_seqid + 1U ||
+	    (stateid_seqid == 1U && state_seqid == 0xffffffffU))
+		nfs_state_log_update_open_stateid(state);
+	else
+		set_bit(NFS_STATE_CHANGE_WAIT, &state->flags);
+}
+
 static void nfs_test_and_clear_all_open_stateid(struct nfs4_state *state)
 {
 	struct nfs_client *clp = state->owner->so_server->nfs_client;
@@ -1393,18 +1412,32 @@ static void nfs_test_and_clear_all_open_stateid(struct nfs4_state *state)
 		nfs4_state_mark_reclaim_nograce(clp, state);
 }
 
+/*
+ * Check for whether or not the caller may update the open stateid
+ * to the value passed in by stateid.
+ *
+ * Note: This function relies heavily on the server implementing
+ * RFC7530 Section 9.1.4.2, and RFC5661 Section 8.2.2
+ * correctly.
+ * i.e. The stateid seqids have to be initialised to 1, and
+ * are then incremented on every state transition.
+ */
 static bool nfs_need_update_open_stateid(struct nfs4_state *state,
-		const nfs4_stateid *stateid, nfs4_stateid *freeme)
+		const nfs4_stateid *stateid)
 {
-	if (test_and_set_bit(NFS_OPEN_STATE, &state->flags) == 0)
-		return true;
-	if (!nfs4_stateid_match_other(stateid, &state->open_stateid)) {
-		nfs4_stateid_copy(freeme, &state->open_stateid);
-		nfs_test_and_clear_all_open_stateid(state);
+	if (test_bit(NFS_OPEN_STATE, &state->flags) == 0 ||
+	    !nfs4_stateid_match_other(stateid, &state->open_stateid)) {
+		if (stateid->seqid == cpu_to_be32(1))
+			nfs_state_log_update_open_stateid(state);
+		else
+			set_bit(NFS_STATE_CHANGE_WAIT, &state->flags);
 		return true;
 	}
-	if (nfs4_stateid_is_newer(stateid, &state->open_stateid))
+
+	if (nfs4_stateid_is_newer(stateid, &state->open_stateid)) {
+		nfs_state_log_out_of_order_open_stateid(state, stateid);
 		return true;
+	}
 	return false;
 }
 
@@ -1443,11 +1476,13 @@ static void nfs_clear_open_stateid_locked(struct nfs4_state *state,
 	if (nfs4_stateid_match_other(stateid, &state->open_stateid) &&
 	    !nfs4_stateid_is_newer(stateid, &state->open_stateid)) {
 		nfs_resync_open_stateid_locked(state);
-		return;
+		goto out;
 	}
 	if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
 		nfs4_stateid_copy(&state->stateid, stateid);
 	nfs4_stateid_copy(&state->open_stateid, stateid);
+out:
+	nfs_state_log_update_open_stateid(state);
 }
 
 static void nfs_clear_open_stateid(struct nfs4_state *state,
@@ -1464,29 +1499,56 @@ static void nfs_clear_open_stateid(struct nfs4_state *state,
 }
 
 static void nfs_set_open_stateid_locked(struct nfs4_state *state,
-		const nfs4_stateid *stateid, fmode_t fmode,
-		nfs4_stateid *freeme)
+		const nfs4_stateid *stateid, nfs4_stateid *freeme)
 {
-	switch (fmode) {
-		case FMODE_READ:
-			set_bit(NFS_O_RDONLY_STATE, &state->flags);
+	DEFINE_WAIT(wait);
+	int status = 0;
+	for (;;) {
+
+		if (!nfs_need_update_open_stateid(state, stateid))
+			return;
+		if (!test_bit(NFS_STATE_CHANGE_WAIT, &state->flags))
 			break;
-		case FMODE_WRITE:
-			set_bit(NFS_O_WRONLY_STATE, &state->flags);
+		if (status)
 			break;
-		case FMODE_READ|FMODE_WRITE:
-			set_bit(NFS_O_RDWR_STATE, &state->flags);
+		/* Rely on seqids for serialisation with NFSv4.0 */
+		if (!nfs4_has_session(NFS_SERVER(state->inode)->nfs_client))
+			break;
+
+		prepare_to_wait(&state->waitq, &wait, TASK_KILLABLE);
+		/*
+		 * Ensure we process the state changes in the same order
+		 * in which the server processed them by delaying the
+		 * update of the stateid until we are in sequence.
+		 */
+		write_sequnlock(&state->seqlock);
+		spin_unlock(&state->owner->so_lock);
+		rcu_read_unlock();
+		if (!signal_pending(current)) {
+			if (schedule_timeout(5*HZ) == 0)
+				status = -EAGAIN;
+			else
+				status = 0;
+		} else
+			status = -EINTR;
+		finish_wait(&state->waitq, &wait);
+		rcu_read_lock();
+		spin_lock(&state->owner->so_lock);
+		write_seqlock(&state->seqlock);
 	}
-	if (!nfs_need_update_open_stateid(state, stateid, freeme))
-		return;
+
+	if (!nfs4_stateid_match_other(stateid, &state->open_stateid)) {
+		nfs4_stateid_copy(freeme, &state->open_stateid);
+		nfs_test_and_clear_all_open_stateid(state);
+	}
+
 	if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
 		nfs4_stateid_copy(&state->stateid, stateid);
 	nfs4_stateid_copy(&state->open_stateid, stateid);
 }
 
-static void __update_open_stateid(struct nfs4_state *state,
+static void nfs_state_set_open_stateid(struct nfs4_state *state,
 		const nfs4_stateid *open_stateid,
-		const nfs4_stateid *deleg_stateid,
 		fmode_t fmode,
 		nfs4_stateid *freeme)
 {
@@ -1494,17 +1556,34 @@ static void __update_open_stateid(struct nfs4_state *state,
 	 * Protect the call to nfs4_state_set_mode_locked and
 	 * serialise the stateid update
 	 */
-	spin_lock(&state->owner->so_lock);
 	write_seqlock(&state->seqlock);
-	if (deleg_stateid != NULL) {
-		nfs4_stateid_copy(&state->stateid, deleg_stateid);
-		set_bit(NFS_DELEGATED_STATE, &state->flags);
+	nfs_set_open_stateid_locked(state, open_stateid, freeme);
+	switch (fmode) {
+	case FMODE_READ:
+		set_bit(NFS_O_RDONLY_STATE, &state->flags);
+		break;
+	case FMODE_WRITE:
+		set_bit(NFS_O_WRONLY_STATE, &state->flags);
+		break;
+	case FMODE_READ|FMODE_WRITE:
+		set_bit(NFS_O_RDWR_STATE, &state->flags);
 	}
-	if (open_stateid != NULL)
-		nfs_set_open_stateid_locked(state, open_stateid, fmode, freeme);
+	set_bit(NFS_OPEN_STATE, &state->flags);
+	write_sequnlock(&state->seqlock);
+}
+
+static void nfs_state_set_delegation(struct nfs4_state *state,
+		const nfs4_stateid *deleg_stateid,
+		fmode_t fmode)
+{
+	/*
+	 * Protect the call to nfs4_state_set_mode_locked and
+	 * serialise the stateid update
+	 */
+	write_seqlock(&state->seqlock);
+	nfs4_stateid_copy(&state->stateid, deleg_stateid);
+	set_bit(NFS_DELEGATED_STATE, &state->flags);
 	write_sequnlock(&state->seqlock);
-	update_open_stateflags(state, fmode);
-	spin_unlock(&state->owner->so_lock);
 }
 
 static int update_open_stateid(struct nfs4_state *state,
@@ -1522,6 +1601,12 @@ static int update_open_stateid(struct nfs4_state *state,
 	fmode &= (FMODE_READ|FMODE_WRITE);
 
 	rcu_read_lock();
+	spin_lock(&state->owner->so_lock);
+	if (open_stateid != NULL) {
+		nfs_state_set_open_stateid(state, open_stateid, fmode, &freeme);
+		ret = 1;
+	}
+
 	deleg_cur = rcu_dereference(nfsi->delegation);
 	if (deleg_cur == NULL)
 		goto no_delegation;
@@ -1538,18 +1623,16 @@ static int update_open_stateid(struct nfs4_state *state,
 		goto no_delegation_unlock;
 
 	nfs_mark_delegation_referenced(deleg_cur);
-	__update_open_stateid(state, open_stateid, &deleg_cur->stateid,
-			fmode, &freeme);
+	nfs_state_set_delegation(state, &deleg_cur->stateid, fmode);
 	ret = 1;
 no_delegation_unlock:
 	spin_unlock(&deleg_cur->lock);
 no_delegation:
+	if (ret)
+		update_open_stateflags(state, fmode);
+	spin_unlock(&state->owner->so_lock);
 	rcu_read_unlock();
 
-	if (!ret && open_stateid != NULL) {
-		__update_open_stateid(state, open_stateid, NULL, fmode, &freeme);
-		ret = 1;
-	}
 	if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags))
 		nfs4_schedule_state_manager(clp);
 	if (freeme.type != 0)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 0378e2257ca7..d615b7cdfa8f 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -645,6 +645,7 @@ nfs4_alloc_open_state(void)
 	INIT_LIST_HEAD(&state->lock_states);
 	spin_lock_init(&state->state_lock);
 	seqlock_init(&state->seqlock);
+	init_waitqueue_head(&state->waitq);
 	return state;
 }
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 02/10] NFSv4: Add a tracepoint to document open stateid updates
  2017-11-06 15:27 ` [PATCH v7 01/10] NFSv4: Fix OPEN / CLOSE race Trond Myklebust
@ 2017-11-06 15:27   ` Trond Myklebust
  2017-11-06 15:27     ` [PATCH v7 03/10] NFSv4: Fix open create exclusive when the server reboots Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c  | 3 +++
 fs/nfs/nfs4trace.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6e3cfa00b4ea..d720b076a34d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1481,6 +1481,7 @@ static void nfs_clear_open_stateid_locked(struct nfs4_state *state,
 	if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
 		nfs4_stateid_copy(&state->stateid, stateid);
 	nfs4_stateid_copy(&state->open_stateid, stateid);
+	trace_nfs4_open_stateid_update(state->inode, stateid, 0);
 out:
 	nfs_state_log_update_open_stateid(state);
 }
@@ -1524,6 +1525,7 @@ static void nfs_set_open_stateid_locked(struct nfs4_state *state,
 		write_sequnlock(&state->seqlock);
 		spin_unlock(&state->owner->so_lock);
 		rcu_read_unlock();
+		trace_nfs4_open_stateid_update_wait(state->inode, stateid, 0);
 		if (!signal_pending(current)) {
 			if (schedule_timeout(5*HZ) == 0)
 				status = -EAGAIN;
@@ -1545,6 +1547,7 @@ static void nfs_set_open_stateid_locked(struct nfs4_state *state,
 	if (test_bit(NFS_DELEGATED_STATE, &state->flags) == 0)
 		nfs4_stateid_copy(&state->stateid, stateid);
 	nfs4_stateid_copy(&state->open_stateid, stateid);
+	trace_nfs4_open_stateid_update(state->inode, stateid, status);
 }
 
 static void nfs_state_set_open_stateid(struct nfs4_state *state,
diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
index e7c6275519b0..bf92cfd627fe 100644
--- a/fs/nfs/nfs4trace.h
+++ b/fs/nfs/nfs4trace.h
@@ -1066,6 +1066,8 @@ DECLARE_EVENT_CLASS(nfs4_inode_stateid_event,
 
 DEFINE_NFS4_INODE_STATEID_EVENT(nfs4_setattr);
 DEFINE_NFS4_INODE_STATEID_EVENT(nfs4_delegreturn);
+DEFINE_NFS4_INODE_STATEID_EVENT(nfs4_open_stateid_update);
+DEFINE_NFS4_INODE_STATEID_EVENT(nfs4_open_stateid_update_wait);
 
 DECLARE_EVENT_CLASS(nfs4_getattr_event,
 		TP_PROTO(
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 03/10] NFSv4: Fix open create exclusive when the server reboots
  2017-11-06 15:27   ` [PATCH v7 02/10] NFSv4: Add a tracepoint to document open stateid updates Trond Myklebust
@ 2017-11-06 15:27     ` Trond Myklebust
  2017-11-06 15:27       ` [PATCH v7 04/10] NFS: Fix a typo in nfs_rename() Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

If the server that does not implement NFSv4.1 persistent session
semantics reboots while we are performing an exclusive create,
then the return value of NFS4ERR_DELAY when we replay the open
during the grace period causes us to lose the verifier.
When the grace period expires, and we present a new verifier,
the server will then correctly reply NFS4ERR_EXIST.

This commit ensures that we always present the same verifier when
replaying the OPEN.

Reported-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4proc.c | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index d720b076a34d..a0bdd1e1e2c1 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1088,6 +1088,12 @@ struct nfs4_opendata {
 	int rpc_status;
 };
 
+struct nfs4_open_createattrs {
+	struct nfs4_label *label;
+	struct iattr *sattr;
+	const __u32 verf[2];
+};
+
 static bool nfs4_clear_cap_atomic_open_v1(struct nfs_server *server,
 		int err, struct nfs4_exception *exception)
 {
@@ -1157,8 +1163,7 @@ static void nfs4_init_opendata_res(struct nfs4_opendata *p)
 
 static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 		struct nfs4_state_owner *sp, fmode_t fmode, int flags,
-		const struct iattr *attrs,
-		struct nfs4_label *label,
+		const struct nfs4_open_createattrs *c,
 		enum open_claim_type4 claim,
 		gfp_t gfp_mask)
 {
@@ -1166,6 +1171,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	struct inode *dir = d_inode(parent);
 	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs_seqid *(*alloc_seqid)(struct nfs_seqid_counter *, gfp_t);
+	struct nfs4_label *label = (c != NULL) ? c->label : NULL;
 	struct nfs4_opendata *p;
 
 	p = kzalloc(sizeof(*p), gfp_mask);
@@ -1231,15 +1237,11 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	case NFS4_OPEN_CLAIM_DELEG_PREV_FH:
 		p->o_arg.fh = NFS_FH(d_inode(dentry));
 	}
-	if (attrs != NULL && attrs->ia_valid != 0) {
-		__u32 verf[2];
-
+	if (c != NULL && c->sattr != NULL && c->sattr->ia_valid != 0) {
 		p->o_arg.u.attrs = &p->attrs;
-		memcpy(&p->attrs, attrs, sizeof(p->attrs));
+		memcpy(&p->attrs, c->sattr, sizeof(p->attrs));
 
-		verf[0] = jiffies;
-		verf[1] = current->pid;
-		memcpy(p->o_arg.u.verifier.data, verf,
+		memcpy(p->o_arg.u.verifier.data, c->verf,
 				sizeof(p->o_arg.u.verifier.data));
 	}
 	p->c_arg.fh = &p->o_res.fh;
@@ -1891,7 +1893,7 @@ static struct nfs4_opendata *nfs4_open_recoverdata_alloc(struct nfs_open_context
 	struct nfs4_opendata *opendata;
 
 	opendata = nfs4_opendata_alloc(ctx->dentry, state->owner, 0, 0,
-			NULL, NULL, claim, GFP_NOFS);
+			NULL, claim, GFP_NOFS);
 	if (opendata == NULL)
 		return ERR_PTR(-ENOMEM);
 	opendata->state = state;
@@ -2822,8 +2824,7 @@ static int _nfs4_open_and_get_state(struct nfs4_opendata *opendata,
 static int _nfs4_do_open(struct inode *dir,
 			struct nfs_open_context *ctx,
 			int flags,
-			struct iattr *sattr,
-			struct nfs4_label *label,
+			const struct nfs4_open_createattrs *c,
 			int *opened)
 {
 	struct nfs4_state_owner  *sp;
@@ -2835,6 +2836,8 @@ static int _nfs4_do_open(struct inode *dir,
 	struct nfs4_threshold **ctx_th = &ctx->mdsthreshold;
 	fmode_t fmode = ctx->mode & (FMODE_READ|FMODE_WRITE|FMODE_EXEC);
 	enum open_claim_type4 claim = NFS4_OPEN_CLAIM_NULL;
+	struct iattr *sattr = c->sattr;
+	struct nfs4_label *label = c->label;
 	struct nfs4_label *olabel = NULL;
 	int status;
 
@@ -2853,8 +2856,8 @@ static int _nfs4_do_open(struct inode *dir,
 	status = -ENOMEM;
 	if (d_really_is_positive(dentry))
 		claim = NFS4_OPEN_CLAIM_FH;
-	opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags, sattr,
-			label, claim, GFP_KERNEL);
+	opendata = nfs4_opendata_alloc(dentry, sp, fmode, flags,
+			c, claim, GFP_KERNEL);
 	if (opendata == NULL)
 		goto err_put_state_owner;
 
@@ -2935,10 +2938,18 @@ static struct nfs4_state *nfs4_do_open(struct inode *dir,
 	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_state *res;
+	struct nfs4_open_createattrs c = {
+		.label = label,
+		.sattr = sattr,
+		.verf = {
+			[0] = (__u32)jiffies,
+			[1] = (__u32)current->pid,
+		},
+	};
 	int status;
 
 	do {
-		status = _nfs4_do_open(dir, ctx, flags, sattr, label, opened);
+		status = _nfs4_do_open(dir, ctx, flags, &c, opened);
 		res = ctx->state;
 		trace_nfs4_open_file(ctx, flags, status);
 		if (status == 0)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 04/10] NFS: Fix a typo in nfs_rename()
  2017-11-06 15:27     ` [PATCH v7 03/10] NFSv4: Fix open create exclusive when the server reboots Trond Myklebust
@ 2017-11-06 15:27       ` Trond Myklebust
  2017-11-06 15:27         ` [PATCH v7 05/10] NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

On successful rename, the "old_dentry" is retained and is attached to
the "new_dir", so we need to call nfs_set_verifier() accordingly.

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

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5ceaeb1f6fb6..9dda2e0ef85e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2064,7 +2064,7 @@ int nfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		 * should mark the directories for revalidation.
 		 */
 		d_move(old_dentry, new_dentry);
-		nfs_set_verifier(new_dentry,
+		nfs_set_verifier(old_dentry,
 					nfs_save_change_attribute(new_dir));
 	} else if (error == -ENOENT)
 		nfs_dentry_handle_enoent(old_dentry);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 05/10] NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.
  2017-11-06 15:27       ` [PATCH v7 04/10] NFS: Fix a typo in nfs_rename() Trond Myklebust
@ 2017-11-06 15:27         ` Trond Myklebust
  2017-11-06 15:27           ` [PATCH v7 06/10] NFSv4: Don't try to CLOSE if the stateid 'other' field has changed Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

If we're racing with an OPEN, then retry the operation instead of
declaring it a success.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
[Andrew W Elble: Fix a typo in nfs4_refresh_open_stateid]
---
 fs/nfs/delegation.c | 27 +++++++++++++++++++++++++++
 fs/nfs/delegation.h |  1 +
 fs/nfs/nfs4_fs.h    |  2 ++
 fs/nfs/nfs4proc.c   | 21 +++++++++++++++++++--
 fs/nfs/nfs4state.c  | 16 ++++++++++++++++
 5 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 606dd3871f66..ade44ca0c66c 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -1041,6 +1041,33 @@ int nfs_delegations_present(struct nfs_client *clp)
 }
 
 /**
+ * nfs4_refresh_delegation_stateid - Update delegation stateid seqid
+ * @dst: stateid to refresh
+ * @inode: inode to check
+ *
+ * Returns "true" and updates "dst->seqid" * if inode had a delegation
+ * that matches our delegation stateid. Otherwise "false" is returned.
+ */
+bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode)
+{
+	struct nfs_delegation *delegation;
+	bool ret = false;
+	if (!inode)
+		goto out;
+
+	rcu_read_lock();
+	delegation = rcu_dereference(NFS_I(inode)->delegation);
+	if (delegation != NULL &&
+	    nfs4_stateid_match_other(dst, &delegation->stateid)) {
+		dst->seqid = delegation->stateid.seqid;
+		return ret;
+	}
+	rcu_read_unlock();
+out:
+	return ret;
+}
+
+/**
  * nfs4_copy_delegation_stateid - Copy inode's state ID information
  * @inode: inode to check
  * @flags: delegation type requirement
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index ddaf2644cf13..185a09f37a89 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -62,6 +62,7 @@ int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4
 int nfs4_open_delegation_recall(struct nfs_open_context *ctx, struct nfs4_state *state, const nfs4_stateid *stateid, fmode_t type);
 int nfs4_lock_delegation_recall(struct file_lock *fl, struct nfs4_state *state, const nfs4_stateid *stateid);
 bool nfs4_copy_delegation_stateid(struct inode *inode, fmode_t flags, nfs4_stateid *dst, struct rpc_cred **cred);
+bool nfs4_refresh_delegation_stateid(nfs4_stateid *dst, struct inode *inode);
 
 void nfs_mark_delegation_referenced(struct nfs_delegation *delegation);
 int nfs4_have_delegation(struct inode *inode, fmode_t flags);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b07124ae5be1..84c3adc6bf96 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -461,6 +461,8 @@ extern int nfs4_set_lock_state(struct nfs4_state *state, struct file_lock *fl);
 extern int nfs4_select_rw_stateid(struct nfs4_state *, fmode_t,
 		const struct nfs_lock_context *, nfs4_stateid *,
 		struct rpc_cred **);
+extern bool nfs4_refresh_open_stateid(nfs4_stateid *dst,
+		struct nfs4_state *state);
 
 extern struct nfs_seqid *nfs_alloc_seqid(struct nfs_seqid_counter *counter, gfp_t gfp_mask);
 extern int nfs_wait_on_sequence(struct nfs_seqid *seqid, struct rpc_task *task);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a0bdd1e1e2c1..153e34c5b6e9 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3198,13 +3198,21 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 
 			}
 			break;
+		case -NFS4ERR_OLD_STATEID:
+			/* Did we race with OPEN? */
+			if (nfs4_refresh_open_stateid(&calldata->arg.stateid,
+						state)) {
+				task->tk_status = 0;
+				rpc_restart_call_prepare(task);
+			}
+			goto out_release;
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_STALE_STATEID:
 		case -NFS4ERR_EXPIRED:
 			nfs4_free_revoked_stateid(server,
 					&calldata->arg.stateid,
 					task->tk_msg.rpc_cred);
-		case -NFS4ERR_OLD_STATEID:
+			/* Fallthrough */
 		case -NFS4ERR_BAD_STATEID:
 			if (!nfs4_stateid_match(&calldata->arg.stateid,
 						&state->open_stateid)) {
@@ -3213,6 +3221,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 			}
 			if (calldata->arg.fmode == 0)
 				break;
+			/* Fallthrough */
 		default:
 			if (nfs4_async_handle_error(task, server, state, NULL) == -EAGAIN) {
 				rpc_restart_call_prepare(task);
@@ -5809,11 +5818,19 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 		nfs4_free_revoked_stateid(data->res.server,
 				data->args.stateid,
 				task->tk_msg.rpc_cred);
+		/* Fallthrough */
 	case -NFS4ERR_BAD_STATEID:
-	case -NFS4ERR_OLD_STATEID:
 	case -NFS4ERR_STALE_STATEID:
 		task->tk_status = 0;
 		break;
+	case -NFS4ERR_OLD_STATEID:
+		if (nfs4_refresh_delegation_stateid(&data->stateid, data->inode)) {
+			task->tk_status = 0;
+			rpc_restart_call_prepare(task);
+			return;
+		}
+		task->tk_status = 0;
+		break;
 	case -NFS4ERR_ACCESS:
 		if (data->args.bitmask) {
 			data->args.bitmask = NULL;
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index d615b7cdfa8f..ccf3b4e03997 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -986,6 +986,22 @@ static int nfs4_copy_lock_stateid(nfs4_stateid *dst,
 	return ret;
 }
 
+bool nfs4_refresh_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
+{
+	bool ret;
+	int seq;
+
+	do {
+		ret = false;
+		seq = read_seqbegin(&state->seqlock);
+		if (nfs4_state_match_open_stateid_other(state, dst)) {
+			dst->seqid = state->open_stateid.seqid;
+			ret = true;
+		}
+	} while (read_seqretry(&state->seqlock, seq));
+	return ret;
+}
+
 static void nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
 {
 	const nfs4_stateid *src;
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 06/10] NFSv4: Don't try to CLOSE if the stateid 'other' field has changed
  2017-11-06 15:27         ` [PATCH v7 05/10] NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID Trond Myklebust
@ 2017-11-06 15:27           ` Trond Myklebust
  2017-11-06 15:27             ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

If the stateid is no longer recognised on the server, either due to a
restart, or due to a competing CLOSE call, then we do not have to
retry. Any open contexts that triggered a reopen of the file, will
also act as triggers for any CLOSE for the updated stateids.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs4_fs.h   |  2 ++
 fs/nfs/nfs4proc.c  | 14 ++++----------
 fs/nfs/nfs4state.c |  9 +++++++--
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 84c3adc6bf96..cdab6e8dbacc 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -463,6 +463,8 @@ extern int nfs4_select_rw_stateid(struct nfs4_state *, fmode_t,
 		struct rpc_cred **);
 extern bool nfs4_refresh_open_stateid(nfs4_stateid *dst,
 		struct nfs4_state *state);
+extern bool nfs4_copy_open_stateid(nfs4_stateid *dst,
+		struct nfs4_state *state);
 
 extern struct nfs_seqid *nfs_alloc_seqid(struct nfs_seqid_counter *counter, gfp_t gfp_mask);
 extern int nfs_wait_on_sequence(struct nfs_seqid *seqid, struct rpc_task *task);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 153e34c5b6e9..fe4a855dc965 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3214,14 +3214,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 					task->tk_msg.rpc_cred);
 			/* Fallthrough */
 		case -NFS4ERR_BAD_STATEID:
-			if (!nfs4_stateid_match(&calldata->arg.stateid,
-						&state->open_stateid)) {
-				rpc_restart_call_prepare(task);
-				goto out_release;
-			}
-			if (calldata->arg.fmode == 0)
-				break;
-			/* Fallthrough */
+			break;
 		default:
 			if (nfs4_async_handle_error(task, server, state, NULL) == -EAGAIN) {
 				rpc_restart_call_prepare(task);
@@ -3253,7 +3246,6 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 	is_rdwr = test_bit(NFS_O_RDWR_STATE, &state->flags);
 	is_rdonly = test_bit(NFS_O_RDONLY_STATE, &state->flags);
 	is_wronly = test_bit(NFS_O_WRONLY_STATE, &state->flags);
-	nfs4_stateid_copy(&calldata->arg.stateid, &state->open_stateid);
 	/* Calculate the change in open mode */
 	calldata->arg.fmode = 0;
 	if (state->n_rdwr == 0) {
@@ -3271,7 +3263,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 		calldata->arg.fmode |= FMODE_READ|FMODE_WRITE;
 
 	if (!nfs4_valid_open_stateid(state) ||
-	    test_bit(NFS_OPEN_STATE, &state->flags) == 0)
+	    !nfs4_refresh_open_stateid(&calldata->arg.stateid, state))
 		call_close = 0;
 	spin_unlock(&state->owner->so_lock);
 
@@ -3365,6 +3357,8 @@ int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait)
 	calldata->inode = state->inode;
 	calldata->state = state;
 	calldata->arg.fh = NFS_FH(state->inode);
+	if (!nfs4_copy_open_stateid(&calldata->arg.stateid, state))
+		goto out_free_calldata;
 	/* Serialization for the sequence id */
 	alloc_seqid = server->nfs_client->cl_mvops->alloc_seqid;
 	calldata->arg.seqid = alloc_seqid(&state->owner->so_seqid, gfp_mask);
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index ccf3b4e03997..4a6f77880992 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1002,18 +1002,23 @@ bool nfs4_refresh_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
 	return ret;
 }
 
-static void nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
+bool nfs4_copy_open_stateid(nfs4_stateid *dst, struct nfs4_state *state)
 {
+	bool ret;
 	const nfs4_stateid *src;
 	int seq;
 
 	do {
+		ret = false;
 		src = &zero_stateid;
 		seq = read_seqbegin(&state->seqlock);
-		if (test_bit(NFS_OPEN_STATE, &state->flags))
+		if (test_bit(NFS_OPEN_STATE, &state->flags)) {
 			src = &state->open_stateid;
+			ret = true;
+		}
 		nfs4_stateid_copy(dst, src);
 	} while (read_seqretry(&state->seqlock, seq));
+	return ret;
 }
 
 /*
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close
  2017-11-06 15:27           ` [PATCH v7 06/10] NFSv4: Don't try to CLOSE if the stateid 'other' field has changed Trond Myklebust
@ 2017-11-06 15:27             ` Trond Myklebust
  2017-11-06 15:27               ` [PATCH v7 08/10] NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn Trond Myklebust
  2017-11-06 19:27               ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Anna Schumaker
  0 siblings, 2 replies; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

If our layoutreturn on close operation returns an NFS4ERR_OLD_STATEID,
then try to update the stateid and retry. We know that there should
be no further LAYOUTGET requests being launched.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index fe4a855dc965..1bb0e405aa57 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3165,11 +3165,18 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 			calldata->arg.lr_args = NULL;
 			calldata->res.lr_res = NULL;
 			break;
+		case -NFS4ERR_OLD_STATEID:
+			if (nfs4_refresh_layout_stateid(&calldata->arg.lr_args->stateid,
+						calldata->inode)) {
+				calldata->res.lr_ret = 0;
+				rpc_restart_call_prepare(task);
+				return;
+			}
+			/* Fallthrough */
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_EXPIRED:
 		case -NFS4ERR_BAD_STATEID:
-		case -NFS4ERR_OLD_STATEID:
 		case -NFS4ERR_UNKNOWN_LAYOUTTYPE:
 		case -NFS4ERR_WRONG_CRED:
 			calldata->arg.lr_args = NULL;
@@ -5787,11 +5794,18 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 			data->args.lr_args = NULL;
 			data->res.lr_res = NULL;
 			break;
+		case -NFS4ERR_OLD_STATEID:
+			if (nfs4_refresh_layout_stateid(&data->args.lr_args->stateid,
+						data->inode)) {
+				data->res.lr_ret = 0;
+				rpc_restart_call_prepare(task);
+				return;
+			}
+			/* Fallthrough */
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_DELEG_REVOKED:
 		case -NFS4ERR_EXPIRED:
 		case -NFS4ERR_BAD_STATEID:
-		case -NFS4ERR_OLD_STATEID:
 		case -NFS4ERR_UNKNOWN_LAYOUTTYPE:
 		case -NFS4ERR_WRONG_CRED:
 			data->args.lr_args = NULL;
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 3bcd669a3152..5e4cd6a7af66 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -355,6 +355,24 @@ pnfs_clear_lseg_state(struct pnfs_layout_segment *lseg,
 }
 
 /*
+ * Update the seqid of a layout stateid
+ */
+bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode)
+{
+	struct pnfs_layout_hdr *lo;
+	bool ret = false;
+
+	spin_lock(&inode->i_lock);
+	lo = NFS_I(inode)->layout;
+	if (lo && nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
+		dst->seqid = lo->plh_stateid.seqid;
+		ret = true;
+	}
+	spin_unlock(&inode->i_lock);
+	return ret;
+}
+
+/*
  * Mark a pnfs_layout_hdr and all associated layout segments as invalid
  *
  * In order to continue using the pnfs_layout_hdr, a full recovery
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 87f144f14d1e..40159cb572c7 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -251,6 +251,7 @@ int pnfs_destroy_layouts_byfsid(struct nfs_client *clp,
 		bool is_recall);
 int pnfs_destroy_layouts_byclid(struct nfs_client *clp,
 		bool is_recall);
+bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode);
 void pnfs_put_layout_hdr(struct pnfs_layout_hdr *lo);
 void pnfs_set_layout_stateid(struct pnfs_layout_hdr *lo,
 			     const nfs4_stateid *new,
@@ -764,6 +765,10 @@ static inline void nfs4_pnfs_v3_ds_connect_unload(void)
 {
 }
 
+bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode)
+{
+	return false;
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 #if IS_ENABLED(CONFIG_NFS_V4_2)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 08/10] NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn
  2017-11-06 15:27             ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Trond Myklebust
@ 2017-11-06 15:27               ` Trond Myklebust
  2017-11-06 15:27                 ` [PATCH v7 09/10] NFSv4: cleanup nfs4_close_done Trond Myklebust
  2017-11-06 19:27               ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Anna Schumaker
  1 sibling, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

If our layoutreturn returns an NFS4ERR_OLD_STATEID, then try to
update the stateid and retry.

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1bb0e405aa57..f11230db8d25 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8766,18 +8766,27 @@ static void nfs4_layoutreturn_done(struct rpc_task *task, void *calldata)
 
 	server = NFS_SERVER(lrp->args.inode);
 	switch (task->tk_status) {
+	case -NFS4ERR_OLD_STATEID:
+		if (nfs4_refresh_layout_stateid(&lrp->args.stateid,
+					lrp->args.inode))
+			goto out_restart;
+		/* Fallthrough */
 	default:
 		task->tk_status = 0;
+		/* Fallthrough */
 	case 0:
 		break;
 	case -NFS4ERR_DELAY:
 		if (nfs4_async_handle_error(task, server, NULL, NULL) != -EAGAIN)
 			break;
-		nfs4_sequence_free_slot(&lrp->res.seq_res);
-		rpc_restart_call_prepare(task);
-		return;
+		goto out_restart;
 	}
 	dprintk("<-- %s\n", __func__);
+	return;
+out_restart:
+	task->tk_status = 0;
+	nfs4_sequence_free_slot(&lrp->res.seq_res);
+	rpc_restart_call_prepare(task);
 }
 
 static void nfs4_layoutreturn_release(void *calldata)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 09/10] NFSv4: cleanup nfs4_close_done
  2017-11-06 15:27               ` [PATCH v7 08/10] NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn Trond Myklebust
@ 2017-11-06 15:27                 ` Trond Myklebust
  2017-11-06 15:27                   ` [PATCH v7 10/10] NFSv4: Clean up nfs4_delegreturn_done Trond Myklebust
  0 siblings, 1 reply; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f11230db8d25..ec62d8ad64ed 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3167,11 +3167,8 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 			break;
 		case -NFS4ERR_OLD_STATEID:
 			if (nfs4_refresh_layout_stateid(&calldata->arg.lr_args->stateid,
-						calldata->inode)) {
-				calldata->res.lr_ret = 0;
-				rpc_restart_call_prepare(task);
-				return;
-			}
+						calldata->inode))
+				goto lr_restart;
 			/* Fallthrough */
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_DELEG_REVOKED:
@@ -3181,9 +3178,7 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 		case -NFS4ERR_WRONG_CRED:
 			calldata->arg.lr_args = NULL;
 			calldata->res.lr_res = NULL;
-			calldata->res.lr_ret = 0;
-			rpc_restart_call_prepare(task);
-			return;
+			goto lr_restart;
 		}
 	}
 
@@ -3199,19 +3194,15 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 			if (calldata->arg.bitmask != NULL) {
 				calldata->arg.bitmask = NULL;
 				calldata->res.fattr = NULL;
-				task->tk_status = 0;
-				rpc_restart_call_prepare(task);
-				goto out_release;
+				goto out_restart;
 
 			}
 			break;
 		case -NFS4ERR_OLD_STATEID:
 			/* Did we race with OPEN? */
 			if (nfs4_refresh_open_stateid(&calldata->arg.stateid,
-						state)) {
-				task->tk_status = 0;
-				rpc_restart_call_prepare(task);
-			}
+						state))
+				goto out_restart;
 			goto out_release;
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_STALE_STATEID:
@@ -3223,17 +3214,23 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
 		case -NFS4ERR_BAD_STATEID:
 			break;
 		default:
-			if (nfs4_async_handle_error(task, server, state, NULL) == -EAGAIN) {
-				rpc_restart_call_prepare(task);
-				goto out_release;
-			}
+			if (nfs4_async_handle_error(task, server, state, NULL) == -EAGAIN)
+				goto out_restart;
 	}
 	nfs_clear_open_stateid(state, &calldata->arg.stateid,
 			res_stateid, calldata->arg.fmode);
 out_release:
+	task->tk_status = 0;
 	nfs_release_seqid(calldata->arg.seqid);
 	nfs_refresh_inode(calldata->inode, &calldata->fattr);
 	dprintk("%s: done, ret = %d!\n", __func__, task->tk_status);
+	return;
+lr_restart:
+	calldata->res.lr_ret = 0;
+out_restart:
+	task->tk_status = 0;
+	rpc_restart_call_prepare(task);
+	goto out_release;
 }
 
 static void nfs4_close_prepare(struct rpc_task *task, void *data)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v7 10/10] NFSv4: Clean up nfs4_delegreturn_done
  2017-11-06 15:27                 ` [PATCH v7 09/10] NFSv4: cleanup nfs4_close_done Trond Myklebust
@ 2017-11-06 15:27                   ` Trond Myklebust
  0 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 15:27 UTC (permalink / raw)
  To: Benjamin Coddington, Anna Schumaker; +Cc: linux-nfs

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ec62d8ad64ed..cbcb27b724bf 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5793,11 +5793,8 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 			break;
 		case -NFS4ERR_OLD_STATEID:
 			if (nfs4_refresh_layout_stateid(&data->args.lr_args->stateid,
-						data->inode)) {
-				data->res.lr_ret = 0;
-				rpc_restart_call_prepare(task);
-				return;
-			}
+						data->inode))
+				goto lr_restart;
 			/* Fallthrough */
 		case -NFS4ERR_ADMIN_REVOKED:
 		case -NFS4ERR_DELEG_REVOKED:
@@ -5807,9 +5804,7 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 		case -NFS4ERR_WRONG_CRED:
 			data->args.lr_args = NULL;
 			data->res.lr_res = NULL;
-			data->res.lr_ret = 0;
-			rpc_restart_call_prepare(task);
-			return;
+			goto lr_restart;
 		}
 	}
 
@@ -5829,29 +5824,30 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 		task->tk_status = 0;
 		break;
 	case -NFS4ERR_OLD_STATEID:
-		if (nfs4_refresh_delegation_stateid(&data->stateid, data->inode)) {
-			task->tk_status = 0;
-			rpc_restart_call_prepare(task);
-			return;
-		}
+		if (nfs4_refresh_delegation_stateid(&data->stateid, data->inode))
+			goto out_restart;
 		task->tk_status = 0;
 		break;
 	case -NFS4ERR_ACCESS:
 		if (data->args.bitmask) {
 			data->args.bitmask = NULL;
 			data->res.fattr = NULL;
-			task->tk_status = 0;
-			rpc_restart_call_prepare(task);
-			return;
+			goto out_restart;
 		}
+		/* Fallthrough */
 	default:
 		if (nfs4_async_handle_error(task, data->res.server,
 					    NULL, NULL) == -EAGAIN) {
-			rpc_restart_call_prepare(task);
-			return;
+			goto out_restart;
 		}
 	}
 	data->rpc_status = task->tk_status;
+	return;
+lr_restart:
+	data->res.lr_ret = 0;
+out_restart:
+	task->tk_status = 0;
+	rpc_restart_call_prepare(task);
 }
 
 static void nfs4_delegreturn_release(void *calldata)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close
  2017-11-06 15:27             ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Trond Myklebust
  2017-11-06 15:27               ` [PATCH v7 08/10] NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn Trond Myklebust
@ 2017-11-06 19:27               ` Anna Schumaker
  2017-11-06 20:26                 ` Trond Myklebust
  1 sibling, 1 reply; 13+ messages in thread
From: Anna Schumaker @ 2017-11-06 19:27 UTC (permalink / raw)
  To: Trond Myklebust, Benjamin Coddington; +Cc: linux-nfs

Hi Trond,

On 11/06/2017 10:27 AM, Trond Myklebust wrote:
> If our layoutreturn on close operation returns an NFS4ERR_OLD_STATEID,
> then try to update the stateid and retry. We know that there should
> be no further LAYOUTGET requests being launched.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  fs/nfs/nfs4proc.c | 18 ++++++++++++++++--
>  fs/nfs/pnfs.c     | 18 ++++++++++++++++++
>  fs/nfs/pnfs.h     |  5 +++++
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index fe4a855dc965..1bb0e405aa57 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3165,11 +3165,18 @@ static void nfs4_close_done(struct rpc_task *task, void *data)
>  			calldata->arg.lr_args = NULL;
>  			calldata->res.lr_res = NULL;
>  			break;
> +		case -NFS4ERR_OLD_STATEID:
> +			if (nfs4_refresh_layout_stateid(&calldata->arg.lr_args->stateid,
> +						calldata->inode)) {
> +				calldata->res.lr_ret = 0;
> +				rpc_restart_call_prepare(task);
> +				return;
> +			}
> +			/* Fallthrough */
>  		case -NFS4ERR_ADMIN_REVOKED:
>  		case -NFS4ERR_DELEG_REVOKED:
>  		case -NFS4ERR_EXPIRED:
>  		case -NFS4ERR_BAD_STATEID:
> -		case -NFS4ERR_OLD_STATEID:
>  		case -NFS4ERR_UNKNOWN_LAYOUTTYPE:
>  		case -NFS4ERR_WRONG_CRED:
>  			calldata->arg.lr_args = NULL;
> @@ -5787,11 +5794,18 @@ static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
>  			data->args.lr_args = NULL;
>  			data->res.lr_res = NULL;
>  			break;
> +		case -NFS4ERR_OLD_STATEID:
> +			if (nfs4_refresh_layout_stateid(&data->args.lr_args->stateid,
> +						data->inode)) {
> +				data->res.lr_ret = 0;
> +				rpc_restart_call_prepare(task);
> +				return;
> +			}
> +			/* Fallthrough */
>  		case -NFS4ERR_ADMIN_REVOKED:
>  		case -NFS4ERR_DELEG_REVOKED:
>  		case -NFS4ERR_EXPIRED:
>  		case -NFS4ERR_BAD_STATEID:
> -		case -NFS4ERR_OLD_STATEID:
>  		case -NFS4ERR_UNKNOWN_LAYOUTTYPE:
>  		case -NFS4ERR_WRONG_CRED:
>  			data->args.lr_args = NULL;
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 3bcd669a3152..5e4cd6a7af66 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -355,6 +355,24 @@ pnfs_clear_lseg_state(struct pnfs_layout_segment *lseg,
>  }
>  
>  /*
> + * Update the seqid of a layout stateid
> + */
> +bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode)
> +{
> +	struct pnfs_layout_hdr *lo;
> +	bool ret = false;
> +
> +	spin_lock(&inode->i_lock);
> +	lo = NFS_I(inode)->layout;
> +	if (lo && nfs4_stateid_match_other(dst, &lo->plh_stateid)) {
> +		dst->seqid = lo->plh_stateid.seqid;
> +		ret = true;
> +	}
> +	spin_unlock(&inode->i_lock);
> +	return ret;
> +}
> +
> +/*
>   * Mark a pnfs_layout_hdr and all associated layout segments as invalid
>   *
>   * In order to continue using the pnfs_layout_hdr, a full recovery
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 87f144f14d1e..40159cb572c7 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -251,6 +251,7 @@ int pnfs_destroy_layouts_byfsid(struct nfs_client *clp,
>  		bool is_recall);
>  int pnfs_destroy_layouts_byclid(struct nfs_client *clp,
>  		bool is_recall);
> +bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode);
>  void pnfs_put_layout_hdr(struct pnfs_layout_hdr *lo);
>  void pnfs_set_layout_stateid(struct pnfs_layout_hdr *lo,
>  			     const nfs4_stateid *new,
> @@ -764,6 +765,10 @@ static inline void nfs4_pnfs_v3_ds_connect_unload(void)
>  {
>  }
>  
> +bool nfs4_refresh_layout_stateid(nfs4_stateid *dst, struct inode *inode)
> +{
> +	return false;
> +}

Shouldn't this be static & inline?  I'm seeing a bunch of "multiple definitions" errors with v4.1 disabled.

Thanks
Anna

>  #endif /* CONFIG_NFS_V4_1 */
>  
>  #if IS_ENABLED(CONFIG_NFS_V4_2)
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close
  2017-11-06 19:27               ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Anna Schumaker
@ 2017-11-06 20:26                 ` Trond Myklebust
  0 siblings, 0 replies; 13+ messages in thread
From: Trond Myklebust @ 2017-11-06 20:26 UTC (permalink / raw)
  To: Anna.Schumaker, bcodding redhat; +Cc: linux-nfs

T24gTW9uLCAyMDE3LTExLTA2IGF0IDE0OjI3IC0wNTAwLCBBbm5hIFNjaHVtYWtlciB3cm90ZToN
Cj4gSGkgVHJvbmQsDQo+IA0KPiBPbiAxMS8wNi8yMDE3IDEwOjI3IEFNLCBUcm9uZCBNeWtsZWJ1
c3Qgd3JvdGU6DQo+ID4gK2Jvb2wgbmZzNF9yZWZyZXNoX2xheW91dF9zdGF0ZWlkKG5mczRfc3Rh
dGVpZCAqZHN0LCBzdHJ1Y3QgaW5vZGUNCj4gPiAqaW5vZGUpDQo+ID4gK3sNCj4gPiArCXJldHVy
biBmYWxzZTsNCj4gPiArfQ0KPiANCj4gU2hvdWxkbid0IHRoaXMgYmUgc3RhdGljICYgaW5saW5l
PyAgSSdtIHNlZWluZyBhIGJ1bmNoIG9mICJtdWx0aXBsZQ0KPiBkZWZpbml0aW9ucyIgZXJyb3Jz
IHdpdGggdjQuMSBkaXNhYmxlZC4NCj4gDQoNCkkgY2xlYXJseSBuZWVkIGEgdmFjYXRpb24uLi4g
4pi6DQpGaW5hbCByZXNlbmQsIEkgaG9wZS4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGlu
dXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVidXN0QHBy
aW1hcnlkYXRhLmNvbQ0K

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-11-06 20:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-06 15:27 [PATCH v7 00/10] Fix OPEN/CLOSE races Trond Myklebust
2017-11-06 15:27 ` [PATCH v7 01/10] NFSv4: Fix OPEN / CLOSE race Trond Myklebust
2017-11-06 15:27   ` [PATCH v7 02/10] NFSv4: Add a tracepoint to document open stateid updates Trond Myklebust
2017-11-06 15:27     ` [PATCH v7 03/10] NFSv4: Fix open create exclusive when the server reboots Trond Myklebust
2017-11-06 15:27       ` [PATCH v7 04/10] NFS: Fix a typo in nfs_rename() Trond Myklebust
2017-11-06 15:27         ` [PATCH v7 05/10] NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID Trond Myklebust
2017-11-06 15:27           ` [PATCH v7 06/10] NFSv4: Don't try to CLOSE if the stateid 'other' field has changed Trond Myklebust
2017-11-06 15:27             ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Trond Myklebust
2017-11-06 15:27               ` [PATCH v7 08/10] NFSv4: Retry NFS4ERR_OLD_STATEID errors in layoutreturn Trond Myklebust
2017-11-06 15:27                 ` [PATCH v7 09/10] NFSv4: cleanup nfs4_close_done Trond Myklebust
2017-11-06 15:27                   ` [PATCH v7 10/10] NFSv4: Clean up nfs4_delegreturn_done Trond Myklebust
2017-11-06 19:27               ` [PATCH v7 07/10] pNFS: Retry NFS4ERR_OLD_STATEID errors in layoutreturn-on-close Anna Schumaker
2017-11-06 20:26                 ` Trond Myklebust

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.