lkml.org 
[lkml]   [2006]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)
On Wed, Dec 20, 2006 at 11:50:50AM -0800, Linus Torvalds wrote:
>
>
> On Wed, 20 Dec 2006, Linus Torvalds wrote:
> >
> > So that's why I've been harping on the fact that I think we simply do
> > really wrong things with PG_dirty at times [ ... ]
>
> Ok, I'll just put my money where my mouth is, and suggest a patch like
> THIS instead.
>
> This one clears up all the issues I find irritating:
>
> - "test_clear_page_dirty()" is insane, both conceptually and as an
> implementation. "Give me a 'C', give me an 'R', give me an 'A', give me
> a 'P'".
>
> So rip out that mindfart entirely.
>
> - "clear_page_dirty()" is badly named, and should be about CANCELLING the
> dirty bit, and must never be called with pages mapped anyway. So throw
> that out too, and replace it with a new function:
>
> void cancel_dirty_page(struct page *page, unsigned int accounting_size);
>
> - "clear_page_dirty_for_io()" is fine.
>
> And with that, I then either rip out any old users of
> "test_clear_page_dirty()" or "clear_page_dirty()", and if appropriate (and
> it's realy lonly appropriate for "truncate()", I replace them with the new
> "cancel_dirty_page()". Most of the time, they should just be deleted
> entirely.
>
> NOTE NOTE NOTE! I _only_ did enough to make things compile for my
> particular configuration. That means that right now the following
> filesystems are broken with this patch (because they use the totally
> broken old crap):
>
> CIFS, FUSE, JFS, ReiserFS, XFS


XFS appears to call clear_page_dirty to get the mapping tree dirty
tag set correctly at the same time the page dirty flag is cleared. I
note that this can be done by set_page_writeback() if we clear the
dirty flag on the page first when we are writing back the entire page.

Hence it seems to me that the XFS call to clear_page_dirty() could
easily be substituted by clear_page_dirty_for_io() followed by a
call to set_page_writeback() to get the mapping tree tags set
correctly after the page has been marked clean.

Does this make sense (even without the posted patch)?

---
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);
Cheers,

Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-12-21 00:29    [from the cache]
©2003-2011 Jasper Spaans