[PATCH 1/5] mm: remove redundant dirty pages check from__delete_from_page_cache()

From: Konstantin Khlebnikov
Date: Mon Jul 08 2013 - 05:52:15 EST


This chunk was added by commit 3a6927906f1b2adf5a31b789322d32eb8559ada0
("Do dirty page accounting when removing a page from the page cache") in 2.6.24.

That was fix for side-effects of commit 3e67c0987d7567ad666641164a153dca9a43b11d
("[PATCH] truncate: clear page dirtiness before running try_to_free_buffers()")
which was required for 46d2277c796f9f4937bfa668c40b2e3f43e93dd0 ("Clean up and
make try_to_free_buffers() not race with dirty pages") and that patch in turn
was reverted by commit ecdfc9787fe527491baefc22dce8b2dbd5b2908d ("Resurrect
'try_to_free_buffers()' VM hackery").

And finally, cancel_dirty_page() was placed after do_invalidatepage in 2.6.25 by
commit a2b345642f530054a92b8d2b5108436225a8093e
("Fix dirty page accounting leak with ext3 data=journal")

So, that hunk is redundant. All other callers of delete_from_page_cache() and
__delete_from_page_cache() handle dirty pages themselves.

I have run xfstest on ext3 in mode data=journal several times. Everything works
fine, except testcase '068' which have triggered deadlock on fs-freeze.
This seems unrelated and probably it's never worked for ext3 in this mode.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxx>
---
mm/filemap.c | 12 ------------
mm/truncate.c | 4 ++++
2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 7905fe7..504aab2 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -135,18 +135,6 @@ void __delete_from_page_cache(struct page *page)
if (PageSwapBacked(page))
__dec_zone_page_state(page, NR_SHMEM);
BUG_ON(page_mapped(page));
-
- /*
- * Some filesystems seem to re-dirty the page even after
- * the VM has canceled the dirty bit (eg ext3 journaling).
- *
- * Fix it up by doing a final dirty accounting check after
- * having removed the page entirely.
- */
- if (PageDirty(page) && mapping_cap_account_dirty(mapping)) {
- dec_zone_page_state(page, NR_FILE_DIRTY);
- dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
- }
}

/**
diff --git a/mm/truncate.c b/mm/truncate.c
index e2e8a8a..9b4721f 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -100,6 +100,10 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
if (page_has_private(page))
do_invalidatepage(page, 0, PAGE_CACHE_SIZE);

+ /*
+ * This is final dirty accounting check. Some filesystems may re-dirty
+ * pages during invalidation, hence it's placed after that.
+ */
cancel_dirty_page(page, PAGE_CACHE_SIZE);

ClearPageMappedToDisk(page);

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