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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20061221012039.GD44411608@melbourne.sgi.com>
Date:	Thu, 21 Dec 2006 12:20:39 +1100
From:	David Chinner <dgc@....com>
To:	Linus Torvalds <torvalds@...l.org>
Cc:	David Chinner <dgc@....com>, Martin Michlmayr <tbm@...ius.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Hugh Dickins <hugh@...itas.com>,
	Nick Piggin <nickpiggin@...oo.com.au>,
	Arjan van de Ven <arjan@...radead.org>,
	Andrei Popa <andrei.popa@...eo.ro>,
	Andrew Morton <akpm@...l.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Florian Weimer <fw@...eb.enyo.de>,
	Marc Haber <mh+linux-kernel@...schlus.de>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Arnd Bergmann <arnd.bergmann@...ibm.com>,
	gordonfarquharson@...il.com
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/