Messages in this thread | | | Date | Thu, 21 Dec 2006 12:20:39 +1100 | From | David Chinner <> | Subject | Re: [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/
| |