public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix dirty page accounting leak with ext3 data=journal
       [not found]                 ` <Pine.LNX.4.64.0712212049001.24477@bizon.gios.gov.pl>
@ 2007-12-21 20:42                   ` Björn Steinbrink
  0 siblings, 0 replies; only message in thread
From: Björn Steinbrink @ 2007-12-21 20:42 UTC (permalink / raw)
  To: Krzysztof Oledzki
  Cc: Linus Torvalds, Andrew Morton, Linux Kernel Mailing List,
	Nick Piggin, Peter Zijlstra, Thomas Osterried, protasnb,
	bugme-daemon, sct, adilget, linux-ext4

In 46d2277c796f9f4937bfa668c40b2e3f43e93dd0, try_to_free_buffers was
changed to bail out if the page was dirty. That caused
truncate_complete_page to leak massive amounts of memory, because the
dirty bit was only cleared after the call to try_to_free_buffers. So the
call to cancel_dirty_page was moved up to have the dirty bit cleared
early in 3e67c0987d7567ad666641164a153dca9a43b11d.

The problem with that fix is, that the page can be redirtied after
cancel_dirty_page was called, eg. like this:

truncate_complete_page()
  cancel_dirty_page() // PG_dirty cleared, decr. dirty pages
  do_invalidatepage()
    ext3_invalidatepage()
      journal_invalidatepage()
        journal_unmap_buffer()
          __dispose_buffer()
            __journal_unfile_buffer()
              __journal_temp_unlink_buffer()
                mark_buffer_dirty(); // PG_dirty set, incr. dirty pages

And then we end up with dirty pages being wrongly accounted.

In ecdfc9787fe527491baefc22dce8b2dbd5b2908d the changes to
try_to_free_buffers were reverted, so the original reason for the
massive memory leak is gone, so we can also revert the move of
the call to cancel_dirty_page from truncate_complete_page and get the
accounting right again.

Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>

---
I'm not sure if it matters, but opposed to the final check in
__remove_from_page_cache, this one also cares about the task io
accounting, so maybe we want to use this instead, although it's not
quite the clean fix either.

diff --git a/mm/truncate.c b/mm/truncate.c
index cadc156..2974903 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -98,11 +98,11 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
 	if (page->mapping != mapping)
 		return;
 
-	cancel_dirty_page(page, PAGE_CACHE_SIZE);
-
 	if (PagePrivate(page))
 		do_invalidatepage(page, 0);
 
+	cancel_dirty_page(page, PAGE_CACHE_SIZE);
+
 	remove_from_page_cache(page);
 	ClearPageUptodate(page);
 	ClearPageMappedToDisk(page);

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2007-12-21 20:42 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <Pine.LNX.4.64.0712152358240.14491@bizon.gios.gov.pl>
     [not found] ` <20071215203539.d6f71e96.akpm@linux-foundation.org>
     [not found]   ` <Pine.LNX.4.64.0712161016290.25065@bizon.gios.gov.pl>
     [not found]     ` <20071216015112.d0ab08a1.akpm@linux-foundation.org>
     [not found]       ` <Pine.LNX.4.64.0712161440030.25065@bizon.gios.gov.pl>
     [not found]         ` <alpine.LFD.0.9999.0712190903350.21557@woody.linux-foundation.org>
     [not found]           ` <20071220141217.GA4745@atjola.homenet>
     [not found]             ` <alpine.LFD.0.9999.0712200753040.21557@woody.linux-foundation.org>
     [not found]               ` <20071220222816.GB4745@atjola.homenet>
     [not found]                 ` <Pine.LNX.4.64.0712212049001.24477@bizon.gios.gov.pl>
2007-12-21 20:42                   ` [PATCH] Fix dirty page accounting leak with ext3 data=journal Björn Steinbrink

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox