NFS: Fix an unstable write data integrity race

Commit 2c61be0a9478258f77b66208a0c4b1f5f8161c3c (NFS: Ensure that the WRITE and COMMIT RPC calls are always uninterruptible) exposed a race on file close. In order to ensure correct close-to-open behaviour, we want to wait for all outstanding background commit operations to complete. This patch adds an inode flag that indicates if a commit operation is under way, and provides a mechanism to allow ->write_inode() to wait for its completion if this is a data integrity flush. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

NFS: Fix an unstable write data integrity race
Commit 2c61be0a9478258f77b66208a0c4b1f5f8161c3c (NFS: Ensure that the WRITE and COMMIT RPC calls are always uninterruptible) exposed a race on file close. In order to ensure correct close-to-open behaviour, we want to wait for all outstanding background commit operations to complete. This patch adds an inode flag that indicates if a commit operation is under way, and provides a mechanism to allow ->write_inode() to wait for its completion if this is a data integrity flush. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Trond Myklebust
1 parent cdd29ecfcb
Showing 2 changed files with 33 additions and 4 deletions Side-by-side Diff
fs/nfs/write.c
include/linux/nfs_fs.h
@@ -1201,6 +1201,25 @@
  
  
 #if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
+static int nfs_commit_set_lock(struct nfs_inode *nfsi, int may_wait)
+{
+	if (!test_and_set_bit(NFS_INO_COMMIT, &nfsi->flags))
+		return 1;
+	if (may_wait && !out_of_line_wait_on_bit_lock(&nfsi->flags,
+				NFS_INO_COMMIT, nfs_wait_bit_killable,
+				TASK_KILLABLE))
+		return 1;
+	return 0;
+}
+
+static void nfs_commit_clear_lock(struct nfs_inode *nfsi)
+{
+	clear_bit(NFS_INO_COMMIT, &nfsi->flags);
+	smp_mb__after_clear_bit();
+	wake_up_bit(&nfsi->flags, NFS_INO_COMMIT);
+}
+
+
 static void nfs_commitdata_release(void *data)
 {
 	struct nfs_write_data *wdata = data;
@@ -1262,8 +1281,6 @@
 	task = rpc_run_task(&task_setup_data);
 	if (IS_ERR(task))
 		return PTR_ERR(task);
-	if (how & FLUSH_SYNC)
-		rpc_wait_for_completion_task(task);
 	rpc_put_task(task);
 	return 0;
 }
@@ -1294,6 +1311,7 @@
 				BDI_RECLAIMABLE);
 		nfs_clear_page_tag_locked(req);
 	}
+	nfs_commit_clear_lock(NFS_I(inode));
 	return -ENOMEM;
 }
  
@@ -1349,6 +1367,7 @@
 	next:
 		nfs_clear_page_tag_locked(req);
 	}
+	nfs_commit_clear_lock(NFS_I(data->inode));
 	nfs_commitdata_release(calldata);
 }
  
  
@@ -1363,8 +1382,11 @@
 static int nfs_commit_inode(struct inode *inode, int how)
 {
 	LIST_HEAD(head);
-	int res;
+	int may_wait = how & FLUSH_SYNC;
+	int res = 0;
  
+	if (!nfs_commit_set_lock(NFS_I(inode), may_wait))
+		goto out;
 	spin_lock(&inode->i_lock);
 	res = nfs_scan_commit(inode, &head, 0, 0);
 	spin_unlock(&inode->i_lock);
@@ -1372,7 +1394,13 @@
 		int error = nfs_commit_list(inode, &head, how);
 		if (error < 0)
 			return error;
-	}
+		if (may_wait)
+			wait_on_bit(&NFS_I(inode)->flags, NFS_INO_COMMIT,
+					nfs_wait_bit_killable,
+					TASK_KILLABLE);
+	} else
+		nfs_commit_clear_lock(NFS_I(inode));
+out:
 	return res;
 }
  
@@ -209,6 +209,7 @@
 #define NFS_INO_FLUSHING	(4)		/* inode is flushing out data */
 #define NFS_INO_FSCACHE		(5)		/* inode can be cached by FS-Cache */
 #define NFS_INO_FSCACHE_LOCK	(6)		/* FS-Cache cookie management lock */
+#define NFS_INO_COMMIT		(7)		/* inode is committing unstable writes */
  
 static inline struct nfs_inode *NFS_I(const struct inode *inode)
 {
...	...	@@ -1201,6 +1201,25 @@
1201	1201
1202	1202
1203	1203	#if defined(CONFIG_NFS_V3) \|\| defined(CONFIG_NFS_V4)
	1204	+static int nfs_commit_set_lock(struct nfs_inode *nfsi, int may_wait)
	1205	+{
	1206	+ if (!test_and_set_bit(NFS_INO_COMMIT, &nfsi->flags))
	1207	+ return 1;
	1208	+ if (may_wait && !out_of_line_wait_on_bit_lock(&nfsi->flags,
	1209	+ NFS_INO_COMMIT, nfs_wait_bit_killable,
	1210	+ TASK_KILLABLE))
	1211	+ return 1;
	1212	+ return 0;
	1213	+}
	1214	+
	1215	+static void nfs_commit_clear_lock(struct nfs_inode *nfsi)
	1216	+{
	1217	+ clear_bit(NFS_INO_COMMIT, &nfsi->flags);
	1218	+ smp_mb__after_clear_bit();
	1219	+ wake_up_bit(&nfsi->flags, NFS_INO_COMMIT);
	1220	+}
	1221	+
	1222	+
1204	1223	static void nfs_commitdata_release(void *data)
1205	1224	{
1206	1225	struct nfs_write_data *wdata = data;
...	...	@@ -1262,8 +1281,6 @@
1262	1281	task = rpc_run_task(&task_setup_data);
1263	1282	if (IS_ERR(task))
1264	1283	return PTR_ERR(task);
1265		- if (how & FLUSH_SYNC)
1266		- rpc_wait_for_completion_task(task);
1267	1284	rpc_put_task(task);
1268	1285	return 0;
1269	1286	}
...	...	@@ -1294,6 +1311,7 @@
1294	1311	BDI_RECLAIMABLE);
1295	1312	nfs_clear_page_tag_locked(req);
1296	1313	}
	1314	+ nfs_commit_clear_lock(NFS_I(inode));
1297	1315	return -ENOMEM;
1298	1316	}
1299	1317
...	...	@@ -1349,6 +1367,7 @@
1349	1367	next:
1350	1368	nfs_clear_page_tag_locked(req);
1351	1369	}
	1370	+ nfs_commit_clear_lock(NFS_I(data->inode));
1352	1371	nfs_commitdata_release(calldata);
1353	1372	}
1354	1373
1355	1374
...	...	@@ -1363,8 +1382,11 @@
1363	1382	static int nfs_commit_inode(struct inode *inode, int how)
1364	1383	{
1365	1384	LIST_HEAD(head);
1366		- int res;
	1385	+ int may_wait = how & FLUSH_SYNC;
	1386	+ int res = 0;
1367	1387
	1388	+ if (!nfs_commit_set_lock(NFS_I(inode), may_wait))
	1389	+ goto out;
1368	1390	spin_lock(&inode->i_lock);
1369	1391	res = nfs_scan_commit(inode, &head, 0, 0);
1370	1392	spin_unlock(&inode->i_lock);
...	...	@@ -1372,7 +1394,13 @@
1372	1394	int error = nfs_commit_list(inode, &head, how);
1373	1395	if (error < 0)
1374	1396	return error;
1375		- }
	1397	+ if (may_wait)
	1398	+ wait_on_bit(&NFS_I(inode)->flags, NFS_INO_COMMIT,
	1399	+ nfs_wait_bit_killable,
	1400	+ TASK_KILLABLE);
	1401	+ } else
	1402	+ nfs_commit_clear_lock(NFS_I(inode));
	1403	+out:
1376	1404	return res;
1377	1405	}
1378	1406
...	...	@@ -209,6 +209,7 @@
209	209	#define NFS_INO_FLUSHING (4) /* inode is flushing out data */
210	210	#define NFS_INO_FSCACHE (5) /* inode can be cached by FS-Cache */
211	211	#define NFS_INO_FSCACHE_LOCK (6) /* FS-Cache cookie management lock */
	212	+#define NFS_INO_COMMIT (7) /* inode is committing unstable writes */
212	213
213	214	static inline struct nfs_inode NFS_I(const struct inode inode)
214	215	{