nfs: fix NR_FILE_DIRTY underflow

PATCH In review Peter Zijlstra 3 participants 9 messages Dec 13, 2006 – Dec 14, 2006

Subsystems (4)

Memory management NFS client MM core Page cache

lore

Share & tools

lore

Peter Zijlstra

author

2006-12-13 12:13 UTC #0 lore raw

PATCH 1/1 nfs: fix NR_FILE_DIRTY underflow

Still testing this patch, but it looks good so far.

---
Just setting PG_dirty can cause NR_FILE_DIRTY to underflow which is bad (TM).

Use set_page_dirty() which will do the right thing.

Signed-off-by Peter Zijlstra <[email protected]>

---
 fs/nfs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/nfs/file.c +1 -1

Index: linux-2.6-git2/fs/nfs/file.c
===================================================================
@@ -321,7 +321,7 @@ static int nfs_release_page(struct page 
 	if (!(gfp & __GFP_FS))
 		return 0;
 	/* Hack... Force nfs_wb_page() to write out the page */
-	SetPageDirty(page);
+	set_page_dirty(page);
 	return !nfs_wb_page(page->mapping->host, page);
 }

Trond Myklebust

2006-12-13 12:26 UTC #1 lore raw

Quoted 7 lines · Peter Zijlstra

Still testing this patch, but it looks good so far.
---
Just setting PG_dirty can cause NR_FILE_DIRTY to underflow which is bad (TM).
Use set_page_dirty() which will do the right thing.

Actually, I'd prefer to have it do the right thing by getting rid of that call to test_clear_page_dirty() inside invalidate_inode_pages2_range(). That is causing loss of data integrity, and is what is causing us to have to hack NFS in the first place.

Cheers
Trond

Quoted 20 lines

Signed-off-by Peter Zijlstra <[email protected]>

---
 fs/nfs/file.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-git2/fs/nfs/file.c
===================================================================

fs/nfs/file.c +1 -1

@@ -321,7 +321,7 @@ static int nfs_release_page(struct page 
 	if (!(gfp & __GFP_FS))
 		return 0;
 	/* Hack... Force nfs_wb_page() to write out the page */
-	SetPageDirty(page);
+	set_page_dirty(page);
 	return !nfs_wb_page(page->mapping->host, page);
 }

Peter Zijlstra

author

2006-12-13 16:22 UTC #2 lore raw

- Sorry for possible duplicates, but I don't seem to be getting to lkml -

Quoted 7 lines · Trond Myklebust

Quoted 6 lines · Peter Zijlstra
Still testing this patch, but it looks good so far.
---
Just setting PG_dirty can cause NR_FILE_DIRTY to underflow which is bad (TM).

Quoted 6 lines

Quoted 1 line
Use set_page_dirty() which will do the right thing.
Actually, I'd prefer to have it do the right thing by getting rid of that call to test_clear_page_dirty() inside invalidate_inode_pages2_range(). That is causing loss of data integrity, and is what is causing us to have to hack NFS in the first place.

Ah, I think I see what your problem is there.
How about this totally untested patch:

(little update - it seems to compile and run, now testing if it fixes
the problem too)

---
Delay clearing the dirty page state till after removing it from the mapping in invalidate_inode_pages2_range(). This will give try_to_release_pages() a shot to flush dirty data.

Signed-off-by Peter Zijlstra <[email protected]>

---
 fs/nfs/file.c              |    2 --
 include/linux/page-flags.h |    2 ++
 mm/page-writeback.c        |   17 +++++++++++------
 mm/truncate.c              |   11 +++--------
 4 files changed, 16 insertions(+), 16 deletions(-)

fs/nfs/file.c +0 -2

Index: linux-2.6-git/fs/nfs/file.c
===================================================================
@@ -320,8 +320,6 @@ static int nfs_release_page(struct page 
 	 */
 	if (!(gfp & __GFP_FS))
 		return 0;
-	/* Hack... Force nfs_wb_page() to write out the page */
-	SetPageDirty(page);
 	return !nfs_wb_page(page_file_mapping(page)->host, page);
 }

include/linux/page-flags.h +2 -0

Index: linux-2.6-git/include/linux/page-flags.h
===================================================================
@@ -252,7 +252,9 @@ static inline void SetPageUptodate(struc
 #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
 
 struct page;	/* forward declaration */
+struct address_space;
 
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page);
 int test_clear_page_dirty(struct page *page);
 int test_clear_page_writeback(struct page *page);
 int test_set_page_writeback(struct page *page);

mm/page-writeback.c +11 -6

Index: linux-2.6-git/mm/page-writeback.c
===================================================================
@@ -850,13 +850,8 @@ int set_page_dirty_lock(struct page *pag
 }
 EXPORT_SYMBOL(set_page_dirty_lock);
 
-/*
- * Clear a page's dirty flag, while caring for dirty memory accounting. 
- * Returns true if the page was previously dirty.
- */
-int test_clear_page_dirty(struct page *page)
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page)
 {
-	struct address_space *mapping = page_mapping(page);
 	unsigned long flags;
 
 	if (!mapping)

@@ -880,6 +875,16 @@ int test_clear_page_dirty(struct page *p
 	write_unlock_irqrestore(&mapping->tree_lock, flags);
 	return 0;
 }
+
+/*
+ * Clear a page's dirty flag, while caring for dirty memory accounting.
+ * Returns true if the page was previously dirty.
+ */
+int test_clear_page_dirty(struct page *page)
+{
+	struct address_space *mapping = page_mapping(page);
+	return __test_clear_page_dirty(mapping, page);
+}
 EXPORT_SYMBOL(test_clear_page_dirty);
 
 /*

mm/truncate.c +3 -8

Index: linux-2.6-git/mm/truncate.c
===================================================================
@@ -307,18 +307,12 @@ invalidate_complete_page2(struct address
 		return 0;
 
 	write_lock_irq(&mapping->tree_lock);
-	if (PageDirty(page))
-		goto failed;
-
 	BUG_ON(PagePrivate(page));
 	__remove_from_page_cache(page);
 	write_unlock_irq(&mapping->tree_lock);
 	ClearPageUptodate(page);
 	page_cache_release(page);	/* pagecache ref */
 	return 1;
-failed:
-	write_unlock_irq(&mapping->tree_lock);
-	return 0;
 }
 
 /**

@@ -386,12 +380,13 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}
-			was_dirty = test_clear_page_dirty(page);
+			was_dirty = PageDirty(page);
 			if (!invalidate_complete_page2(mapping, page)) {
 				if (was_dirty)
 					set_page_dirty(page);
 				ret = -EIO;
-			}
+			} else
+				__test_clear_page_dirty(mapping, page);
 			unlock_page(page);
 		}
 		pagevec_release(&pvec);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Peter Zijlstra

author

2006-12-13 17:11 UTC #3 lore raw

In reply to Trond Myklebust

Quoted 7 lines · Trond Myklebust

Quoted 6 lines · Peter Zijlstra
Still testing this patch, but it looks good so far.
---
Just setting PG_dirty can cause NR_FILE_DIRTY to underflow which is bad (TM).

Quoted 6 lines

Quoted 1 line
Use set_page_dirty() which will do the right thing.
Actually, I'd prefer to have it do the right thing by getting rid of that call to test_clear_page_dirty() inside invalidate_inode_pages2_range(). That is causing loss of data integrity, and is what is causing us to have to hack NFS in the first place.

Ah, I think I see what your problem is there.
How about this totally untested patch:

---
Delay clearing the dirty page state till after removing it from the mapping in invalidate_inode_pages2_range(). This will give try_to_release_pages() a shot to flush dirty data.

Signed-off-by Peter Zijlstra <[email protected]>

---
 fs/nfs/file.c              |    2 --
 include/linux/page-flags.h |    2 ++
 mm/page-writeback.c        |   17 +++++++++++------
 mm/truncate.c              |   11 +++--------
 4 files changed, 16 insertions(+), 16 deletions(-)

fs/nfs/file.c +0 -2

Index: linux-2.6-git/fs/nfs/file.c
===================================================================
@@ -320,8 +320,6 @@ static int nfs_release_page(struct page 
 	 */
 	if (!(gfp & __GFP_FS))
 		return 0;
-	/* Hack... Force nfs_wb_page() to write out the page */
-	SetPageDirty(page);
 	return !nfs_wb_page(page_file_mapping(page)->host, page);
 }

include/linux/page-flags.h +2 -0

Index: linux-2.6-git/include/linux/page-flags.h
===================================================================
@@ -252,7 +252,9 @@ static inline void SetPageUptodate(struc
 #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
 
 struct page;	/* forward declaration */
+struct address_space;
 
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page);
 int test_clear_page_dirty(struct page *page);
 int test_clear_page_writeback(struct page *page);
 int test_set_page_writeback(struct page *page);

mm/page-writeback.c +11 -6

Index: linux-2.6-git/mm/page-writeback.c
===================================================================
@@ -850,13 +850,8 @@ int set_page_dirty_lock(struct page *pag
 }
 EXPORT_SYMBOL(set_page_dirty_lock);
 
-/*
- * Clear a page's dirty flag, while caring for dirty memory accounting. 
- * Returns true if the page was previously dirty.
- */
-int test_clear_page_dirty(struct page *page)
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page)
 {
-	struct address_space *mapping = page_mapping(page);
 	unsigned long flags;
 
 	if (!mapping)

@@ -880,6 +875,16 @@ int test_clear_page_dirty(struct page *p
 	write_unlock_irqrestore(&mapping->tree_lock, flags);
 	return 0;
 }
+
+/*
+ * Clear a page's dirty flag, while caring for dirty memory accounting.
+ * Returns true if the page was previously dirty.
+ */
+int test_clear_page_dirty(struct page *page)
+{
+	struct address_space *mapping = page_mapping(page);
+	return __test_clear_page_dirty(mapping, page);
+}
 EXPORT_SYMBOL(test_clear_page_dirty);
 
 /*

mm/truncate.c +3 -8

Index: linux-2.6-git/mm/truncate.c
===================================================================
@@ -307,18 +307,12 @@ invalidate_complete_page2(struct address
 		return 0;
 
 	write_lock_irq(&mapping->tree_lock);
-	if (PageDirty(page))
-		goto failed;
-
 	BUG_ON(PagePrivate(page));
 	__remove_from_page_cache(page);
 	write_unlock_irq(&mapping->tree_lock);
 	ClearPageUptodate(page);
 	page_cache_release(page);	/* pagecache ref */
 	return 1;
-failed:
-	write_unlock_irq(&mapping->tree_lock);
-	return 0;
 }
 
 /**

@@ -386,12 +380,13 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}
-			was_dirty = test_clear_page_dirty(page);
+			was_dirty = PageDirty(page);
 			if (!invalidate_complete_page2(mapping, page)) {
 				if (was_dirty)
 					set_page_dirty(page);
 				ret = -EIO;
-			}
+			} else
+				__test_clear_page_dirty(mapping, page);
 			unlock_page(page);
 		}
 		pagevec_release(&pvec);

Trond Myklebust

2006-12-13 17:40 UTC #4 lore raw

Quoted 118 lines · Peter Zijlstra

Quoted 7 lines · Trond Myklebust
Quoted 6 lines · Peter Zijlstra
Still testing this patch, but it looks good so far.
---
Just setting PG_dirty can cause NR_FILE_DIRTY to underflow which is bad (TM).
Quoted 6 lines
Quoted 1 line
Use set_page_dirty() which will do the right thing.
Actually, I'd prefer to have it do the right thing by getting rid of that call to test_clear_page_dirty() inside invalidate_inode_pages2_range(). That is causing loss of data integrity, and is what is causing us to have to hack NFS in the first place.
Ah, I think I see what your problem is there.
How about this totally untested patch:
---
Delay clearing the dirty page state till after removing it from the mapping in invalidate_inode_pages2_range(). This will give try_to_release_pages() a shot to flush dirty data.
Signed-off-by Peter Zijlstra <[email protected]>
---
 fs/nfs/file.c              |    2 --
 include/linux/page-flags.h |    2 ++
 mm/page-writeback.c        |   17 +++++++++++------
 mm/truncate.c              |   11 +++--------
 4 files changed, 16 insertions(+), 16 deletions(-)
Index: linux-2.6-git/fs/nfs/file.c
===================================================================
fs/nfs/file.c +0 -2
@@ -320,8 +320,6 @@ static int nfs_release_page(struct page 
 	 */
 	if (!(gfp & __GFP_FS))
 		return 0;
-	/* Hack... Force nfs_wb_page() to write out the page */
-	SetPageDirty(page);
 	return !nfs_wb_page(page_file_mapping(page)->host, page);
 }
 
Index: linux-2.6-git/include/linux/page-flags.h
===================================================================
include/linux/page-flags.h +2 -0
@@ -252,7 +252,9 @@ static inline void SetPageUptodate(struc
 #define ClearPageUncached(page)	clear_bit(PG_uncached, &(page)->flags)
 
 struct page;	/* forward declaration */
+struct address_space;
 
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page);
 int test_clear_page_dirty(struct page *page);
 int test_clear_page_writeback(struct page *page);
 int test_set_page_writeback(struct page *page);
Index: linux-2.6-git/mm/page-writeback.c
===================================================================
mm/page-writeback.c +11 -6
@@ -850,13 +850,8 @@ int set_page_dirty_lock(struct page *pag
 }
 EXPORT_SYMBOL(set_page_dirty_lock);
 
-/*
- * Clear a page's dirty flag, while caring for dirty memory accounting. 
- * Returns true if the page was previously dirty.
- */
-int test_clear_page_dirty(struct page *page)
+int __test_clear_page_dirty(struct address_space *mapping, struct page *page)
 {
-	struct address_space *mapping = page_mapping(page);
 	unsigned long flags;
 
 	if (!mapping)
@@ -880,6 +875,16 @@ int test_clear_page_dirty(struct page *p
 	write_unlock_irqrestore(&mapping->tree_lock, flags);
 	return 0;
 }
+
+/*
+ * Clear a page's dirty flag, while caring for dirty memory accounting.
+ * Returns true if the page was previously dirty.
+ */
+int test_clear_page_dirty(struct page *page)
+{
+	struct address_space *mapping = page_mapping(page);
+	return __test_clear_page_dirty(mapping, page);
+}
 EXPORT_SYMBOL(test_clear_page_dirty);
 
 /*
Index: linux-2.6-git/mm/truncate.c
===================================================================
mm/truncate.c +0 -6
@@ -307,18 +307,12 @@ invalidate_complete_page2(struct address
 		return 0;
 
 	write_lock_irq(&mapping->tree_lock);
-	if (PageDirty(page))
-		goto failed;
-
 	BUG_ON(PagePrivate(page));
 	__remove_from_page_cache(page);
 	write_unlock_irq(&mapping->tree_lock);
 	ClearPageUptodate(page);
 	page_cache_release(page);	/* pagecache ref */
 	return 1;
-failed:
-	write_unlock_irq(&mapping->tree_lock);
-	return 0;
 }
 
 /**

mm/truncate.c int invalidate_inode_pages2_range(struct · line 386

Quoted 8 lines

mm/truncate.c +1 -1

@@ -386,12 +380,13 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}
-			was_dirty = test_clear_page_dirty(page);
+			was_dirty = PageDirty(page);
 			if (!invalidate_complete_page2(mapping, page)) {
 				if (was_dirty)
 					set_page_dirty(page);

                                          ^^^^^^^^^^^^^^^^^^^^^

Why would you still need this?

mm/truncate.c int invalidate_inode_pages2_range(struct · line 392

Quoted 8 lines

mm/truncate.c +2 -1

 				ret = -EIO;
-			}
+			} else
+				__test_clear_page_dirty(mapping, page);
 			unlock_page(page);
 		}
 		pagevec_release(&pvec);

Cheers
Trond

Peter Zijlstra

author

2006-12-13 18:51 UTC #5 lore raw

Quoted 13 lines · Trond Myklebust

Review int invalidate_inode_pages2_range(struct · line 386

Quoted 9 lines · Peter Zijlstra

(quoted diff) +1 -1

@@ -386,12 +380,13 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}
-			was_dirty = test_clear_page_dirty(page);
+			was_dirty = PageDirty(page);
 			if (!invalidate_complete_page2(mapping, page)) {
 				if (was_dirty)
 					set_page_dirty(page);

                                          ^^^^^^^^^^^^^^^^^^^^^

Quoted 1 line

Why would you still need this?

Hmm yeah, that doesn't make sense. What kind of twist did I get my brain into.

This should do.

Hmm, does this agree with the DIO path? I just noticed generic_file_direct_IO() also uses invalidate_inode_pages2_range(). I think it does, its looks like all they want is to ensure nothing remains in the pagecache after a write, which itself ensures nothing is dirty to begin with.

---
Delay clearing the dirty page state till after we've invalidated the page in invalidate_complete_page2(). This gives try_to_release_pages() a chance to flush dirty data.

Signed-off-by Peter Zijlstra <[email protected]>

---
 fs/nfs/file.c |    2 --
 mm/truncate.c |   14 ++------------
 2 files changed, 2 insertions(+), 14 deletions(-)

fs/nfs/file.c +0 -2

Index: linux-2.6-git/fs/nfs/file.c
===================================================================
@@ -320,8 +320,6 @@ static int nfs_release_page(struct page 
 	 */
 	if (!(gfp & __GFP_FS))
 		return 0;
-	/* Hack... Force nfs_wb_page() to write out the page */
-	set_page_dirty(page);
 	return !nfs_wb_page(page_file_mapping(page)->host, page);
 }

mm/truncate.c +2 ~~-12~~

Index: linux-2.6-git/mm/truncate.c
===================================================================
@@ -306,19 +306,14 @@ invalidate_complete_page2(struct address
 	if (PagePrivate(page) && !try_to_release_page(page, GFP_KERNEL))
 		return 0;
 
+	test_clear_page_dirty(page);
 	write_lock_irq(&mapping->tree_lock);
-	if (PageDirty(page))
-		goto failed;
-
 	BUG_ON(PagePrivate(page));
 	__remove_from_page_cache(page);
 	write_unlock_irq(&mapping->tree_lock);
 	ClearPageUptodate(page);
 	page_cache_release(page);	/* pagecache ref */
 	return 1;
-failed:
-	write_unlock_irq(&mapping->tree_lock);
-	return 0;
 }
 
 /**

@@ -350,7 +345,6 @@ int invalidate_inode_pages2_range(struct
 		for (i = 0; !ret && i < pagevec_count(&pvec); i++) {
 			struct page *page = pvec.pages[i];
 			pgoff_t page_index;
-			int was_dirty;
 
 			lock_page(page);
 			if (page->mapping != mapping) {

@@ -386,12 +380,8 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}
-			was_dirty = test_clear_page_dirty(page);
-			if (!invalidate_complete_page2(mapping, page)) {
-				if (was_dirty)
-					set_page_dirty(page);
+			if (!invalidate_complete_page2(mapping, page))
 				ret = -EIO;
-			}
 			unlock_page(page);
 		}
 		pagevec_release(&pvec);

Andrew Morton

2006-12-14 01:29 UTC #6 lore raw

Quoted 35 lines · On Wed, 13 Dec 2006 19:48:34 +0100 Peter Zijlstra <[email protected]>…

mm/truncate.c +1 -7

@@ -306,19 +306,14 @@ invalidate_complete_page2(struct address
 	if (PagePrivate(page) && !try_to_release_page(page, GFP_KERNEL))
 		return 0;
 
+	test_clear_page_dirty(page);
 	write_lock_irq(&mapping->tree_lock);
-	if (PageDirty(page))
-		goto failed;
-
 	BUG_ON(PagePrivate(page));
 	__remove_from_page_cache(page);
 	write_unlock_irq(&mapping->tree_lock);
 	ClearPageUptodate(page);
 	page_cache_release(page);	/* pagecache ref */
 	return 1;
-failed:
-	write_unlock_irq(&mapping->tree_lock);
-	return 0;
 }
 
 /**

@@ -350,7 +345,6 @@ int invalidate_inode_pages2_range(struct
 		for (i = 0; !ret && i < pagevec_count(&pvec); i++) {
 			struct page *page = pvec.pages[i];
 			pgoff_t page_index;
-			int was_dirty;
 
 			lock_page(page);
 			if (page->mapping != mapping) {

@@ -386,12 +380,8 @@ int invalidate_inode_pages2_range(struct
 					  PAGE_CACHE_SIZE, 0);
 				}
 			}

mm/truncate.c int invalidate_inode_pages2_range(struct · line 387

Quoted 10 lines

mm/truncate.c +1 -5

-			was_dirty = test_clear_page_dirty(page);
-			if (!invalidate_complete_page2(mapping, page)) {
-				if (was_dirty)
-					set_page_dirty(page);
+			if (!invalidate_complete_page2(mapping, page))
 				ret = -EIO;
-			}
 			unlock_page(page);
 		}
 		pagevec_release(&pvec);

a) we're now calling try_to_release_page() with a potentially-dirty page, whereas it was previously clean.

I wouldn't expect ->releasepage() implementations to go looking at PG_Dirty, because that's not what they're suppoed to be interested in. But they might do, dunno.

b) If invalidate_complete_page2() failed due to, say, dirty buffer_heads then we now have a clean page with dirty buffers. That is an illegal state and the page will leak permanently.

I _think_ that's what the was_dirty logic is in there for: to preserve the correct page-vs-buffers dirtiness coherency. But I'd need to do some 2.5.x changelog-dumpster-diving to be sure.

Sigh. I don't know what invalidate_inode_pages2() is *supposed to do*.

What are its semantics wrt unfreeable page metadata? Against page dirtiness?

It was written for direct-io and had one set of semantics for that, then NFS came along and seemed to require a slightly different set of semantics, even though the earlier semantics weren't really defined, leading to a belief that the present semantics are "wrong", without a definition of what semantics NFS actually desires.

So let's start again.

Trond, please define precisely and completely and without reference to the existing implementation: what behaviour does NFS want?

Andrew Morton

2006-12-14 01:41 UTC #7 lore raw

Quoted 6 lines · On Wed, 13 Dec 2006 17:29:21 -0800 Andrew Morton <[email protected]> wrote:

a) we're now calling try_to_release_page() with a potentially-dirty page, whereas it was previously clean.
I wouldn't expect ->releasepage() implementations to go looking at PG_Dirty, because that's not what they're suppoed to be interested in. But they might do, dunno.

Still an issue, probably minor.

Quoted 7 lines

b) If invalidate_complete_page2() failed due to, say, dirty buffer_heads then we now have a clean page with dirty buffers. That is an illegal state and the page will leak permanently.
I _think_ that's what the was_dirty logic is in there for: to preserve the correct page-vs-buffers dirtiness coherency. But I'd need to do some 2.5.x changelog-dumpster-diving to be sure.

no, that's bs. The patch looks OK from that POV: try_to_release_page() will be able to clear clean buffers from a dirty page.

And in fact if it did that, it will then clean the page for us (see test_clear_page_dirty() in try_to_free_buffers()).

But we still need the clear_page_dirty() in invalidate_complete_page2() in case we didn't call try_to_release_page() at all.

Quoted 2 lines

Trond, please define precisely and completely and without reference to the existing implementation: what behaviour does NFS want?

But this would be nice.

Trond Myklebust

2006-12-14 14:52 UTC #8 lore raw

Quoted 2 lines · Andrew Morton

Quoted 2 lines
Trond, please define precisely and completely and without reference to the existing implementation: what behaviour does NFS want?

Part of the behaviour is dictated by our needs for invalidate_inode_pages2(), part of the behaviour is dictated by the 2-stage NFS writeout mechanism.

Starting with invalidate_inode_pages2(). The goal here is to have a function that is guaranteed to force all currently cached pages to be read in from the server afresh. In other words, we'd like to throw out the old mapping of the file, and start with a new one. However, we also need to guarantee that any modifications that a user may have made to the file are preserved. Throwing out a dirty page is forbidden unless the data has been written to disk first.
What we're doing in try_to_release_page() in the current implementation of invalidate_inode_pages2() is therefore to fix races: the goal is to ensure that we write back any dirty data before the page is removed. This is made necessary by the fact that the VM may at any time override our call to unmap_mapping_range() and allow the page to be re-dirtied by the user.

Next: about the 2-stage NFS writeout mechanism. A client may want to allow the server to cache writeback data for a while, so that it can flush several WRITE RPC calls to disk at the same time, maximising I/O efficiency in terms of elevator algorithms etc.
It signals this to the server by means of the 'unstable' flag on the WRITE RPC call, which allows the former to just write the data into its pagecache. Before the client can actually free up the page it still needs to know that the data it wrote to the server has been flushed from the pagecache onto disk, and this is done by sending the COMMIT RPC request. The latter acts rather like fdatasync() does on local data: it guarantees that all file data within a specified range has been flushed to disk. Failure of the COMMIT request signals to the client that the server may have rebooted, and so the page needs to be written out again. Our second use of try_to_release_page() is therefore to ensure that the server has indeed flushed that dirty data from its pagecache to its disk before we allow the VM to release the page on the client. We don't send a COMMIT request for each and every page that is to be released, but we do ensure that at least one COMMIT has been sent that covers the data range represented by the page to be freed.

Cheers
Trond