Gitweb:     
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=a2b345642f530054a92b8d2b5108436225a8093e
Commit:     a2b345642f530054a92b8d2b5108436225a8093e
Parent:     ae1276b9349a2fd9c3afb4651e25a77ac03299d9
Author:     Bjorn Steinbrink <[EMAIL PROTECTED]>
AuthorDate: Mon Feb 4 22:29:28 2008 -0800
Committer:  Linus Torvalds <[EMAIL PROTECTED]>
CommitDate: Tue Feb 5 09:44:19 2008 -0800

    Fix dirty page accounting leak with ext3 data=journal
    
    In 46d2277c796f9f4937bfa668c40b2e3f43e93dd0 ("Clean up and make
    try_to_free_buffers() not race with dirty pages"), try_to_free_buffers
    was changed to bail out if the page was dirty.
    
    That in turn 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 ("truncate:
    clear page dirtiness before running try_to_free_buffers()").
    
    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.
    
    As a result, in ecdfc9787fe527491baefc22dce8b2dbd5b2908d ("Resurrect
    'try_to_free_buffers()' VM hackery") the changes to try_to_free_buffers
    were reverted, so the original reason for the massive memory leak is
    gone, and we can also revert the move of the call to cancel_dirty_page
    from truncate_complete_page and get the accounting right again.
    
    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.
    
    Signed-off-by: Björn Steinbrink <[EMAIL PROTECTED]>
    Tested-by: Krzysztof Piotr Oledzki <[EMAIL PROTECTED]>
    Cc: Jan Kara <[EMAIL PROTECTED]>
    Cc: Nick Piggin <[EMAIL PROTECTED]>
    Cc: Peter Zijlstra <[EMAIL PROTECTED]>
    Cc: Thomas Osterried <[EMAIL PROTECTED]>
    Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
    Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
---
 mm/truncate.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/truncate.c b/mm/truncate.c
index 3855492..9838c05 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);
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to