Review: xfs_start_page_writeback should use clear_page_dirty_for_io

Review: xfs_start_page_writeback should use clear_page_dirty_for_io

[David Chinner]

As has been discuss on LKML w.r.t to an ext3 corruption bug
(http://lkml.org/lkml/2006/12/16/164), we should not use clear_page_dirty()
as it can result in inconsistent state within the VM and is likely
to go away very soon. Instead, we should be using clear_page_dirty_for_io()
which does the right thing. Some references:

http://lkml.org/lkml/2006/12/20/204
http://lkml.org/lkml/2006/12/20/295
http://lkml.org/lkml/2006/12/20/310
http://lkml.org/lkml/2006/12/20/362

Linus's patch fixes the corruption seen on ARM, so is likely to be merged
(potentially as a stable 2.6.19.x fix).

That means we rely on set_page_writeback() to set the tag bits in the mapping
tree based on whether the page is dirty or not, so we have to call that
_after_ we call clear_page_dirty_for_io() instead of before.

Comments?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

---
 fs/xfs/linux-2.6/xfs_aops.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- 2.6.x-xfs-new.orig/fs/xfs/linux-2.6/xfs_aops.c	2006-12-19 12:22:47.000000000 +1100
+++ 2.6.x-xfs-new/fs/xfs/linux-2.6/xfs_aops.c	2006-12-21 10:15:04.545375877 +1100
@@ -340,9 +340,9 @@ xfs_start_page_writeback(
 {
 	ASSERT(PageLocked(page));
 	ASSERT(!PageWriteback(page));
-	set_page_writeback(page);
 	if (clear_dirty)
-		clear_page_dirty(page);
+		clear_page_dirty_for_io(page);
+	set_page_writeback(page);
 	unlock_page(page);
 	if (!buffers) {
 		end_page_writeback(page);

Re: Review: xfs_start_page_writeback should use clear_page_dirty_for_io

[David Chinner]

On Fri, Dec 22, 2006 at 10:51:46AM +1100, David Chinner wrote:
> As has been discuss on LKML w.r.t to an ext3 corruption bug
> (http://lkml.org/lkml/2006/12/16/164), we should not use clear_page_dirty()
> as it can result in inconsistent state within the VM and is likely
> to go away very soon. Instead, we should be using clear_page_dirty_for_io()
> which does the right thing. Some references:
> 
> http://lkml.org/lkml/2006/12/20/204
> http://lkml.org/lkml/2006/12/20/295
> http://lkml.org/lkml/2006/12/20/310
> http://lkml.org/lkml/2006/12/20/362
> 
> Linus's patch fixes the corruption seen on ARM, so is likely to be merged
> (potentially as a stable 2.6.19.x fix).

FYI, Linus has already commited his patches and this patch to his git tree.

http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=fba2591bf4e418b6c3f9f8794c9dd8fe40ae7bd9
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=921320210bd2ec4f17053d283355b73048ac0e56

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group