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

[Linus Torvalds <torvalds@osdl.org>]

On Wed, 20 Dec 2006, Andrew Morton wrote:
> > 
> > So with my change, afaik, we will just return EIO to the invalidate, and 
> > do the write.
> 
> The write's already been done by this stage.

Ok, but the end result is the same: you MUST NOT just "cancel" a write. It 
needs to be done, or the backing store must be actually de-allocated. You 
can't just say "get rid of it" and think that it can work. Exactly because 
of security issues, and just the simple fact that reading it back gets 
random contents.

So I repeat: clearing a dirty bit really only has two valid cases. Not 
three, like we used to have. And the "cancel" case cannot be conditional: 
either you can cancel it or you cannot. There's no

	if (cancel_dirty_page()) {
			..

sequence that makes sense that I can think of.

> > It really boils down to that same thing: if you remove the dirty bit, 
> > there is NO CONCEIVABLE GOOD THING YOU CAN DO EXCEPT FOR:
> >  - do the damn IO already ("clear_page_dirty_for_io()")
> >  - truncate the page (unmap and destroy it both from page cache AND from 
> >    any user-visible filesystem cases)
> 
> There's also redirty_page_for_writepage().

_dirtying_ a page makes sense in any situation. You can always dirty them. 
I'm just saying that you can't just mark them *clean*.

If your point was that the filesystem had better be able to take care of 
"redirty_page_for_writepage()", then yes, of course. But since it's the 
filesystem itself that does it, it had _better_ be able to take care of 
the situation it puts itself into.

			Linus
-
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/