Re: 2.6.19 file content corruption on ext3

[Linus Torvalds]

On Tue, 19 Dec 2006, Nick Piggin wrote:
> > 
> > Anyway it has the same issues as the others. See what happens when you
> > run two test_clear_page_dirty_sync_ptes() consecutively, you still loose
> > PG_dirty even though the page might actually be dirty.
> 
> How can this happen? We'll only test_clear_page_dirty_sync_ptes again
> after buffers have been reattached, and subsequently cleaned. And in
> that case if the ptes are still clean at this point then the page really
> is clean.

Why do you talk about buffers being reattached? Are you still in some 
world where "try_to_free_buffers()" matters? Have you not followed the 
discussion? Why do you ignore my MUCH SIMPLER patch that just removed all 
this crap ENTIRELY from "try_to_free_buffers()", and the exact same 
corruption happened?

Forget about "try_to_free_buffers()". Please apply this patch to your tree 
first. That gets rid of _one_ copy of totally insane code that did all the 
wrong things.

Only after you have applied this patch should you look at the code again. 
Realizing that the corruption still happens.

So forget about buffers already. That piece of code was crap.

		Linus

---
diff --git a/fs/buffer.c b/fs/buffer.c
index d1f1b54..263f88e 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2834,7 +2834,7 @@ int try_to_free_buffers(struct page *page)
 	int ret = 0;
 
 	BUG_ON(!PageLocked(page));
-	if (PageWriteback(page))
+	if (PageDirty(page) || PageWriteback(page))
 		return 0;
 
 	if (mapping == NULL) {		/* can this still happen? */
@@ -2845,22 +2845,6 @@ int try_to_free_buffers(struct page *page)
 	spin_lock(&mapping->private_lock);
 	ret = drop_buffers(page, &buffers_to_free);
 	spin_unlock(&mapping->private_lock);
-	if (ret) {
-		/*
-		 * If the filesystem writes its buffers by hand (eg ext3)
-		 * then we can have clean buffers against a dirty page.  We
-		 * clean the page here; otherwise later reattachment of buffers
-		 * could encounter a non-uptodate page, which is unresolvable.
-		 * This only applies in the rare case where try_to_free_buffers
-		 * succeeds but the page is not freed.
-		 *
-		 * Also, during truncate, discard_buffer will have marked all
-		 * the page's buffers clean.  We discover that here and clean
-		 * the page also.
-		 */
-		if (test_clear_page_dirty(page))
-			task_io_account_cancelled_write(PAGE_CACHE_SIZE);
-	}
 out:
 	if (buffers_to_free) {
 		struct buffer_head *bh = buffers_to_free;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/