lkml.org 
[lkml]   [2006]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)
On Wed, Dec 20, 2006 at 03:55:25PM -0800, Linus Torvalds wrote:
> On Thu, 21 Dec 2006, David Chinner wrote:
> >
> > 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.
>
> Yes. I think the XFS routine should just use "clear_page_dirty_fir_io()",
> since that matches what it actually wants to do (surprise surprise, it's
> going to write it out).

Yup ;)

> HOWEVER. Why is it conditional? Can somebody who understands XFS tell me
> why "clear_dirty" would ever be 0? I can grep the sources, and I see that
> it's an unconditional 1 in one call-site, but then in the other one it
> does
>
> xfs_start_page_writeback(page, wbc, !page_dirty, count);

page dirty starts at the number of dirty buffers on the page, and as
we map each dirty buffer into the I/O we decrement the page dirty count.

Hence if we map all of the buffers into the I/O, we are cleaning
the entire page and hence we can clear the dirty state on the page.

> and that part just blows my mind. Why would you do a
> xfs_start_page_writeback() and _not_ write the page out? Is this for a
> partial-page-only case?

Yes, partial-page-only case when doing speculative write clustering. We'll hit
this when an extent boundary is not page aligned (fs block size < page size
case) and we need to issue at least two separate I/Os to clean the page.
Because we leave the page dirty and we are working ahead of the index in
generic_writepages() we'll get the rest of the page flushed when we return
back to generic_writepages() as the page is still dirty in the mapping
tree....

> Anyway, your patch looks fine. It seems to be the right thing to do.

Ok, thanks, Linus.

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 02:25    [W:0.581 / U:0.380 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site