Re: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)

[David Chinner <dgc@sgi.com>]

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/