From: "Björn Steinbrink" <B.Steinbrink@gmx.de>
To: Krzysztof Oledzki <olel@ans.pl>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Nick Piggin <nickpiggin@yahoo.com.au>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Osterried <osterried@jesse.de>,
protasnb@gmail.com, bugme-daemon@bugzilla.kernel.org,
sct@redhat.com, adilget@clusterfs.com,
linux-ext4@vger.kernel.org
Subject: [PATCH] Fix dirty page accounting leak with ext3 data=journal
Date: Fri, 21 Dec 2007 21:42:13 +0100 [thread overview]
Message-ID: <20071221204213.GA32138@atjola.homenet> (raw)
In-Reply-To: <Pine.LNX.4.64.0712212049001.24477@bizon.gios.gov.pl>
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);
prev parent reply other threads:[~2007-12-21 20:42 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20071215221935.306A5108068@picon.linux-foundation.org>
2007-12-15 23:08 ` [Bug 9182] Critical memory leak (dirty pages) Krzysztof Oledzki
2007-12-16 4:35 ` Andrew Morton
2007-12-16 9:33 ` Krzysztof Oledzki
2007-12-16 9:51 ` Andrew Morton
2007-12-16 13:46 ` Krzysztof Oledzki
2007-12-16 21:51 ` Andrew Morton
2007-12-17 14:26 ` Jan Kara
2007-12-17 17:17 ` Krzysztof Oledzki
2007-12-19 17:44 ` Linus Torvalds
2007-12-20 1:05 ` Jan Kara
2007-12-20 1:19 ` Nick Piggin
2007-12-20 14:12 ` Björn Steinbrink
2007-12-20 15:04 ` Jan Kara
2007-12-20 16:05 ` Jan Kara
2007-12-20 16:25 ` Linus Torvalds
2007-12-20 17:25 ` Jan Kara
2007-12-20 19:24 ` Linus Torvalds
2007-12-21 1:59 ` Nick Piggin
2007-12-20 22:28 ` Björn Steinbrink
2007-12-21 19:59 ` Krzysztof Oledzki
2007-12-21 20:42 ` Björn Steinbrink [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071221204213.GA32138@atjola.homenet \
--to=b.steinbrink@gmx.de \
--cc=adilget@clusterfs.com \
--cc=akpm@linux-foundation.org \
--cc=bugme-daemon@bugzilla.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nickpiggin@yahoo.com.au \
--cc=olel@ans.pl \
--cc=osterried@jesse.de \
--cc=peterz@infradead.org \
--cc=protasnb@gmail.com \
--cc=sct@redhat.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.