NFS: Allow redirtying of a completed unstable write.

Currently, if an unstable write completes, we cannot redirty the page in order to reflect a new change in the page data until after we've sent a COMMIT request. This patch allows a page rewrite to proceed without the unnecessary COMMIT step, putting it immediately back onto the dirty page list, undoing the VM unstable write accounting, and removing the NFS_PAGE_TAG_COMMIT tag from the NFS radix tree. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

NFS: Allow redirtying of a completed unstable write.
Currently, if an unstable write completes, we cannot redirty the page in order to reflect a new change in the page data until after we've sent a COMMIT request. This patch allows a page rewrite to proceed without the unnecessary COMMIT step, putting it immediately back onto the dirty page list, undoing the VM unstable write accounting, and removing the NFS_PAGE_TAG_COMMIT tag from the NFS radix tree. Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Trond Myklebust
1 parent e7d39069e3
Showing 2 changed files with 38 additions and 36 deletions Side-by-side Diff
fs/nfs/write.c
include/linux/nfs_page.h
@@ -242,12 +242,9 @@
 			return ret;
 		spin_lock(&inode->i_lock);
 	}
-	if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
-		/* This request is marked for commit */
+	if (test_bit(PG_CLEAN, &req->wb_flags)) {
 		spin_unlock(&inode->i_lock);
-		nfs_clear_page_tag_locked(req);
-		nfs_pageio_complete(pgio);
-		return 0;
+		BUG();
 	}
 	if (nfs_set_page_writeback(page) != 0) {
 		spin_unlock(&inode->i_lock);
@@ -391,19 +388,6 @@
 	__set_page_dirty_nobuffers(req->wb_page);
 }
  
-/*
- * Check if a request is dirty
- */
-static inline int
-nfs_dirty_request(struct nfs_page *req)
-{
-	struct page *page = req->wb_page;
-
-	if (page == NULL || test_bit(PG_NEED_COMMIT, &req->wb_flags))
-		return 0;
-	return !PageWriteback(page);
-}
-
 #if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
 /*
  * Add a request to the inode's commit list.
@@ -416,7 +400,7 @@
  
 	spin_lock(&inode->i_lock);
 	nfsi->ncommit++;
-	set_bit(PG_NEED_COMMIT, &(req)->wb_flags);
+	set_bit(PG_CLEAN, &(req)->wb_flags);
 	radix_tree_tag_set(&nfsi->nfs_page_tree,
 			req->wb_index,
 			NFS_PAGE_TAG_COMMIT);
@@ -426,6 +410,19 @@
 	__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
 }
  
+static int
+nfs_clear_request_commit(struct nfs_page *req)
+{
+	struct page *page = req->wb_page;
+
+	if (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
+		dec_zone_page_state(page, NR_UNSTABLE_NFS);
+		dec_bdi_stat(page->mapping->backing_dev_info, BDI_RECLAIMABLE);
+		return 1;
+	}
+	return 0;
+}
+
 static inline
 int nfs_write_need_commit(struct nfs_write_data *data)
 {
@@ -435,7 +432,7 @@
 static inline
 int nfs_reschedule_unstable_write(struct nfs_page *req)
 {
-	if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
+	if (test_and_clear_bit(PG_NEED_COMMIT, &req->wb_flags)) {
 		nfs_mark_request_commit(req);
 		return 1;
 	}
@@ -451,6 +448,12 @@
 {
 }
  
+static inline int
+nfs_clear_request_commit(struct nfs_page *req)
+{
+	return 0;
+}
+
 static inline
 int nfs_write_need_commit(struct nfs_write_data *data)
 {
  
@@ -508,11 +511,8 @@
  
 	while(!list_empty(head)) {
 		req = nfs_list_entry(head->next);
-		dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
-		dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
-				BDI_RECLAIMABLE);
 		nfs_list_remove_request(req);
-		clear_bit(PG_NEED_COMMIT, &(req)->wb_flags);
+		nfs_clear_request_commit(req);
 		nfs_inode_remove_request(req);
 		nfs_unlock_request(req);
 	}
@@ -584,8 +584,7 @@
 		 * Note: nfs_flush_incompatible() will already
 		 * have flushed out requests having wrong owners.
 		 */
-		if (!nfs_dirty_request(req)
-		    || offset > rqend
+		if (offset > rqend
 		    || end < req->wb_offset)
 			goto out_flushme;
  
@@ -601,6 +600,10 @@
 		spin_lock(&inode->i_lock);
 	}
  
+	if (nfs_clear_request_commit(req))
+		radix_tree_tag_clear(&NFS_I(inode)->nfs_page_tree,
+				req->wb_index, NFS_PAGE_TAG_COMMIT);
+
 	/* Okay, the request matches. Update the region */
 	if (offset < req->wb_offset) {
 		req->wb_offset = offset;
@@ -682,8 +685,7 @@
 		req = nfs_page_find_request(page);
 		if (req == NULL)
 			return 0;
-		do_flush = req->wb_page != page || req->wb_context != ctx
-			|| !nfs_dirty_request(req);
+		do_flush = req->wb_page != page || req->wb_context != ctx;
 		nfs_release_request(req);
 		if (!do_flush)
 			return 0;
@@ -1288,10 +1290,7 @@
 	while (!list_empty(&data->pages)) {
 		req = nfs_list_entry(data->pages.next);
 		nfs_list_remove_request(req);
-		clear_bit(PG_NEED_COMMIT, &(req)->wb_flags);
-		dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
-		dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
-				BDI_RECLAIMABLE);
+		nfs_clear_request_commit(req);
  
 		dprintk("NFS:       commit (%s/%lld %d@%lld)",
 			req->wb_context->path.dentry->d_inode->i_sb->s_id,
@@ -1467,7 +1466,7 @@
 		req = nfs_page_find_request(page);
 		if (req == NULL)
 			goto out;
-		if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
+		if (test_bit(PG_CLEAN, &req->wb_flags)) {
 			nfs_release_request(req);
 			break;
 		}
@@ -27,9 +27,12 @@
 /*
  * Valid flags for a dirty buffer
  */
-#define PG_BUSY			0
-#define PG_NEED_COMMIT		1
-#define PG_NEED_RESCHED		2
+enum {
+	PG_BUSY = 0,
+	PG_CLEAN,
+	PG_NEED_COMMIT,
+	PG_NEED_RESCHED,
+};
  
 struct nfs_inode;
 struct nfs_page {
...	...	@@ -242,12 +242,9 @@
242	242	return ret;
243	243	spin_lock(&inode->i_lock);
244	244	}
245		- if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
246		- /* This request is marked for commit */
	245	+ if (test_bit(PG_CLEAN, &req->wb_flags)) {
247	246	spin_unlock(&inode->i_lock);
248		- nfs_clear_page_tag_locked(req);
249		- nfs_pageio_complete(pgio);
250		- return 0;
	247	+ BUG();
251	248	}
252	249	if (nfs_set_page_writeback(page) != 0) {
253	250	spin_unlock(&inode->i_lock);
...	...	@@ -391,19 +388,6 @@
391	388	__set_page_dirty_nobuffers(req->wb_page);
392	389	}
393	390
394		-/*
395		- * Check if a request is dirty
396		- */
397		-static inline int
398		-nfs_dirty_request(struct nfs_page *req)
399		-{
400		- struct page *page = req->wb_page;
401		-
402		- if (page == NULL \|\| test_bit(PG_NEED_COMMIT, &req->wb_flags))
403		- return 0;
404		- return !PageWriteback(page);
405		-}
406		-
407	391	#if defined(CONFIG_NFS_V3) \|\| defined(CONFIG_NFS_V4)
408	392	/*
409	393	* Add a request to the inode's commit list.
...	...	@@ -416,7 +400,7 @@
416	400
417	401	spin_lock(&inode->i_lock);
418	402	nfsi->ncommit++;
419		- set_bit(PG_NEED_COMMIT, &(req)->wb_flags);
	403	+ set_bit(PG_CLEAN, &(req)->wb_flags);
420	404	radix_tree_tag_set(&nfsi->nfs_page_tree,
421	405	req->wb_index,
422	406	NFS_PAGE_TAG_COMMIT);
...	...	@@ -426,6 +410,19 @@
426	410	__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
427	411	}
428	412
	413	+static int
	414	+nfs_clear_request_commit(struct nfs_page *req)
	415	+{
	416	+ struct page *page = req->wb_page;
	417	+
	418	+ if (test_and_clear_bit(PG_CLEAN, &(req)->wb_flags)) {
	419	+ dec_zone_page_state(page, NR_UNSTABLE_NFS);
	420	+ dec_bdi_stat(page->mapping->backing_dev_info, BDI_RECLAIMABLE);
	421	+ return 1;
	422	+ }
	423	+ return 0;
	424	+}
	425	+
429	426	static inline
430	427	int nfs_write_need_commit(struct nfs_write_data *data)
431	428	{
...	...	@@ -435,7 +432,7 @@
435	432	static inline
436	433	int nfs_reschedule_unstable_write(struct nfs_page *req)
437	434	{
438		- if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
	435	+ if (test_and_clear_bit(PG_NEED_COMMIT, &req->wb_flags)) {
439	436	nfs_mark_request_commit(req);
440	437	return 1;
441	438	}
...	...	@@ -451,6 +448,12 @@
451	448	{
452	449	}
453	450
	451	+static inline int
	452	+nfs_clear_request_commit(struct nfs_page *req)
	453	+{
	454	+ return 0;
	455	+}
	456	+
454	457	static inline
455	458	int nfs_write_need_commit(struct nfs_write_data *data)
456	459	{
457	460
...	...	@@ -508,11 +511,8 @@
508	511
509	512	while(!list_empty(head)) {
510	513	req = nfs_list_entry(head->next);
511		- dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
512		- dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
513		- BDI_RECLAIMABLE);
514	514	nfs_list_remove_request(req);
515		- clear_bit(PG_NEED_COMMIT, &(req)->wb_flags);
	515	+ nfs_clear_request_commit(req);
516	516	nfs_inode_remove_request(req);
517	517	nfs_unlock_request(req);
518	518	}
...	...	@@ -584,8 +584,7 @@
584	584	* Note: nfs_flush_incompatible() will already
585	585	* have flushed out requests having wrong owners.
586	586	*/
587		- if (!nfs_dirty_request(req)
588		- \|\| offset > rqend
	587	+ if (offset > rqend
589	588	\|\| end < req->wb_offset)
590	589	goto out_flushme;
591	590
...	...	@@ -601,6 +600,10 @@
601	600	spin_lock(&inode->i_lock);
602	601	}
603	602
	603	+ if (nfs_clear_request_commit(req))
	604	+ radix_tree_tag_clear(&NFS_I(inode)->nfs_page_tree,
	605	+ req->wb_index, NFS_PAGE_TAG_COMMIT);
	606	+
604	607	/* Okay, the request matches. Update the region */
605	608	if (offset < req->wb_offset) {
606	609	req->wb_offset = offset;
...	...	@@ -682,8 +685,7 @@
682	685	req = nfs_page_find_request(page);
683	686	if (req == NULL)
684	687	return 0;
685		- do_flush = req->wb_page != page \|\| req->wb_context != ctx
686		- \|\| !nfs_dirty_request(req);
	688	+ do_flush = req->wb_page != page \|\| req->wb_context != ctx;
687	689	nfs_release_request(req);
688	690	if (!do_flush)
689	691	return 0;
...	...	@@ -1288,10 +1290,7 @@
1288	1290	while (!list_empty(&data->pages)) {
1289	1291	req = nfs_list_entry(data->pages.next);
1290	1292	nfs_list_remove_request(req);
1291		- clear_bit(PG_NEED_COMMIT, &(req)->wb_flags);
1292		- dec_zone_page_state(req->wb_page, NR_UNSTABLE_NFS);
1293		- dec_bdi_stat(req->wb_page->mapping->backing_dev_info,
1294		- BDI_RECLAIMABLE);
	1293	+ nfs_clear_request_commit(req);
1295	1294
1296	1295	dprintk("NFS: commit (%s/%lld %d@%lld)",
1297	1296	req->wb_context->path.dentry->d_inode->i_sb->s_id,
...	...	@@ -1467,7 +1466,7 @@
1467	1466	req = nfs_page_find_request(page);
1468	1467	if (req == NULL)
1469	1468	goto out;
1470		- if (test_bit(PG_NEED_COMMIT, &req->wb_flags)) {
	1469	+ if (test_bit(PG_CLEAN, &req->wb_flags)) {
1471	1470	nfs_release_request(req);
1472	1471	break;
1473	1472	}
...	...	@@ -27,9 +27,12 @@
27	27	/*
28	28	* Valid flags for a dirty buffer
29	29	*/
30		-#define PG_BUSY 0
31		-#define PG_NEED_COMMIT 1
32		-#define PG_NEED_RESCHED 2
	30	+enum {
	31	+ PG_BUSY = 0,
	32	+ PG_CLEAN,
	33	+ PG_NEED_COMMIT,
	34	+ PG_NEED_RESCHED,
	35	+};
33	36
34	37	struct nfs_inode;
35	38	struct nfs_page {