xfs
[Top] [All Lists]

Review: xfs_start_page_writeback should use clear_page_dirty_for_io

To: xfs-dev@xxxxxxx
Subject: Review: xfs_start_page_writeback should use clear_page_dirty_for_io
From: David Chinner <dgc@xxxxxxx>
Date: Fri, 22 Dec 2006 10:51:46 +1100
Cc: xfs@xxxxxxxxxxx
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.4.2.1i
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);


<Prev in Thread] Current Thread [Next in Thread>