Fix OPEN/CLOSE races

Patches to fix up the race between OPEN and CLOSE.

v2:
- Fix a sleep-while-atomic issue
- Clean up.
- Add a tracepoint to help document wait incidents. v3:
- Fix a state update issue

Trond Myklebust (2):
NFSv4: Fix OPEN / CLOSE race
NFSv4: Add a tracepoint to document open stateid updates

 fs/nfs/nfs4_fs.h   |   1 +
 fs/nfs/nfs4proc.c  | 135 +++++++++++++++++++++++++++++++++++++++++------------
 fs/nfs/nfs4trace.h |   2 +
 3 files changed, 109 insertions(+), 29 deletions(-)

2.13.6

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.

---
 fs/nfs/nfs4_fs.h  |   1 +
 fs/nfs/nfs4proc.c | 132 ++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 104 insertions(+), 29 deletions(-)

index b547d935aaf0..46aeaa2ee700 100644
@@ -161,6 +161,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 {

2.13.6

---
 fs/nfs/nfs4proc.c  | 3 +++
 fs/nfs/nfs4trace.h | 2 ++
 2 files changed, 5 insertions(+)

index 9045f167c1b5..c6cbea42da97 100644
@@ -1488,6 +1488,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);
 }

@@ -1536,6 +1537,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);
 		status = wait_on_bit_timeout(&state->flags,
 				NFS_STATE_CHANGE_WAIT,
 				TASK_KILLABLE, 5*HZ);

@@ -1546,6 +1548,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);
 	nfs_state_log_update_open_stateid(state);
 }
 

index be1da19c65d6..b9962d93e746 100644
@@ -1065,6 +1065,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

T24gRnJpLCAyMDE3LTEwLTI3IGF0IDE2OjE4IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6 DQo+IFBhdGNoZXMgdG8gZml4IHVwIHRoZSByYWNlIGJldHdlZW4gT1BFTiBhbmQgQ0xPU0UuDQo+ IA0KPiB2MjoNCj4gLSBGaXggYSBzbGVlcC13aGlsZS1hdG9taWMgaXNzdWUNCj4gLSBDbGVhbiB1 cC4gICAgDQo+IC0gQWRkIGEgdHJhY2Vwb2ludCB0byBoZWxwIGRvY3VtZW50IHdhaXQgaW5jaWRl bnRzLg0KPiB2MzoNCj4gLSBGaXggYSBzdGF0ZSB1cGRhdGUgaXNzdWUNCj4gDQo+IFRyb25kIE15 a2xlYnVzdCAoMik6DQo+ICAgTkZTdjQ6IEZpeCBPUEVOIC8gQ0xPU0UgcmFjZQ0KPiAgIE5GU3Y0 OiBBZGQgYSB0cmFjZXBvaW50IHRvIGRvY3VtZW50IG9wZW4gc3RhdGVpZCB1cGRhdGVzDQo+IA0K PiAgZnMvbmZzL25mczRfZnMuaCAgIHwgICAxICsNCj4gIGZzL25mcy9uZnM0cHJvYy5jICB8IDEz NSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKw0KPiAtLS0tLS0tLS0t LS0NCj4gIGZzL25mcy9uZnM0dHJhY2UuaCB8ICAgMiArDQo+ICAzIGZpbGVzIGNoYW5nZWQsIDEw OSBpbnNlcnRpb25zKCspLCAyOSBkZWxldGlvbnMoLSkNCj4gDQoNCnY0IGNvbWluZyB1cCwgaG9w ZWZ1bGx5IGxhdGVyIHRvZGF5LiBJJ20gc2VlaW5nIGFuIGlzc3VlIHdpdGggeGZzdGVzdHMNCmdl bmVyaWMvMDEzIHdoZXJlIHRoZSBjbGllbnQga2VlcHMgdGhpbmtpbmcgdGhlIHNlcnZlciBpcyBz Y3Jld2VkIHVwLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWlu dGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbQ0K