提交 b9ea2515 编写于 作者: K Konstantin Khlebnikov 提交者: Linus Torvalds

page_writeback: clean up mess around cancel_dirty_page()

This patch replaces cancel_dirty_page() with a helper function
account_page_cleaned() which only updates counters.  It's called from
truncate_complete_page() and from try_to_free_buffers() (hack for ext3).
Page is locked in both cases, page-lock protects against concurrent
dirtiers: see commit 2d6d7f98 ("mm: protect set_page_dirty() from
ongoing truncation").

Delete_from_page_cache() shouldn't be called for dirty pages, they must
be handled by caller (either written or truncated).  This patch treats
final dirty accounting fixup at the end of __delete_from_page_cache() as
a debug check and adds WARN_ON_ONCE() around it.  If something removes
dirty pages without proper handling that might be a bug and unwritten
data might be lost.

Hugetlbfs has no dirty pages accounting, ClearPageDirty() is enough
here.

cancel_dirty_page() in nfs_wb_page_cancel() is redundant.  This is
helper for nfs_invalidate_page() and it's called only in case complete
invalidation.

The mess was started in v2.6.20 after commits 46d2277c ("Clean up
and make try_to_free_buffers() not race with dirty pages") and
3e67c098 ("truncate: clear page dirtiness before running
try_to_free_buffers()") first was reverted right in v2.6.20 in commit
ecdfc978 ("Resurrect 'try_to_free_buffers()' VM hackery"), second in
v2.6.25 commit a2b34564 ("Fix dirty page accounting leak with ext3
data=journal").

Custom fixes were introduced between these points.  NFS in v2.6.23, commit
1b3b4a1a ("NFS: Fix a write request leak in nfs_invalidate_page()").
Kludge in __delete_from_page_cache() in v2.6.24, commit 3a692790 ("Do
dirty page accounting when removing a page from the page cache").  Since
v2.6.25 all of them are redundant.

[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: NKonstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc: Tejun Heo <tj@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: NAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>
上级 ca0984ca
...@@ -55,7 +55,9 @@ truncate_complete_page(struct address_space *mapping, struct page *page) ...@@ -55,7 +55,9 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
if (PagePrivate(page)) if (PagePrivate(page))
page->mapping->a_ops->invalidatepage(page, 0, PAGE_CACHE_SIZE); page->mapping->a_ops->invalidatepage(page, 0, PAGE_CACHE_SIZE);
cancel_dirty_page(page, PAGE_SIZE); if (TestClearPageDirty(page))
account_page_cleaned(page, mapping);
ClearPageMappedToDisk(page); ClearPageMappedToDisk(page);
ll_delete_from_page_cache(page); ll_delete_from_page_cache(page);
} }
......
...@@ -3243,8 +3243,8 @@ int try_to_free_buffers(struct page *page) ...@@ -3243,8 +3243,8 @@ int try_to_free_buffers(struct page *page)
* to synchronise against __set_page_dirty_buffers and prevent the * to synchronise against __set_page_dirty_buffers and prevent the
* dirty bit from being lost. * dirty bit from being lost.
*/ */
if (ret) if (ret && TestClearPageDirty(page))
cancel_dirty_page(page, PAGE_CACHE_SIZE); account_page_cleaned(page, mapping);
spin_unlock(&mapping->private_lock); spin_unlock(&mapping->private_lock);
out: out:
if (buffers_to_free) { if (buffers_to_free) {
......
...@@ -319,7 +319,7 @@ static int hugetlbfs_write_end(struct file *file, struct address_space *mapping, ...@@ -319,7 +319,7 @@ static int hugetlbfs_write_end(struct file *file, struct address_space *mapping,
static void truncate_huge_page(struct page *page) static void truncate_huge_page(struct page *page)
{ {
cancel_dirty_page(page, /* No IO accounting for huge pages? */0); ClearPageDirty(page);
ClearPageUptodate(page); ClearPageUptodate(page);
delete_from_page_cache(page); delete_from_page_cache(page);
} }
......
...@@ -1876,11 +1876,6 @@ int nfs_wb_page_cancel(struct inode *inode, struct page *page) ...@@ -1876,11 +1876,6 @@ int nfs_wb_page_cancel(struct inode *inode, struct page *page)
* request from the inode / page_private pointer and * request from the inode / page_private pointer and
* release it */ * release it */
nfs_inode_remove_request(req); nfs_inode_remove_request(req);
/*
* In case nfs_inode_remove_request has marked the
* page as being dirty
*/
cancel_dirty_page(page, PAGE_CACHE_SIZE);
nfs_unlock_and_release_request(req); nfs_unlock_and_release_request(req);
} }
......
...@@ -1294,9 +1294,11 @@ int __set_page_dirty_no_writeback(struct page *page); ...@@ -1294,9 +1294,11 @@ int __set_page_dirty_no_writeback(struct page *page);
int redirty_page_for_writepage(struct writeback_control *wbc, int redirty_page_for_writepage(struct writeback_control *wbc,
struct page *page); struct page *page);
void account_page_dirtied(struct page *page, struct address_space *mapping); void account_page_dirtied(struct page *page, struct address_space *mapping);
void account_page_cleaned(struct page *page, struct address_space *mapping);
int set_page_dirty(struct page *page); int set_page_dirty(struct page *page);
int set_page_dirty_lock(struct page *page); int set_page_dirty_lock(struct page *page);
int clear_page_dirty_for_io(struct page *page); int clear_page_dirty_for_io(struct page *page);
int get_cmdline(struct task_struct *task, char *buffer, int buflen); int get_cmdline(struct task_struct *task, char *buffer, int buflen);
/* Is the vma a continuation of the stack vma above it? */ /* Is the vma a continuation of the stack vma above it? */
......
...@@ -328,8 +328,6 @@ static inline void SetPageUptodate(struct page *page) ...@@ -328,8 +328,6 @@ static inline void SetPageUptodate(struct page *page)
CLEARPAGEFLAG(Uptodate, uptodate) CLEARPAGEFLAG(Uptodate, uptodate)
extern void cancel_dirty_page(struct page *page, unsigned int account_size);
int test_clear_page_writeback(struct page *page); int test_clear_page_writeback(struct page *page);
int __test_set_page_writeback(struct page *page, bool keep_write); int __test_set_page_writeback(struct page *page, bool keep_write);
......
...@@ -203,16 +203,15 @@ void __delete_from_page_cache(struct page *page, void *shadow) ...@@ -203,16 +203,15 @@ void __delete_from_page_cache(struct page *page, void *shadow)
BUG_ON(page_mapped(page)); BUG_ON(page_mapped(page));
/* /*
* Some filesystems seem to re-dirty the page even after * At this point page must be either written or cleaned by truncate.
* the VM has canceled the dirty bit (eg ext3 journaling). * Dirty page here signals a bug and loss of unwritten data.
* *
* Fix it up by doing a final dirty accounting check after * This fixes dirty accounting after removing the page entirely but
* having removed the page entirely. * leaves PageDirty set: it has no effect for truncated page and
* anyway will be cleared before returning page into buddy allocator.
*/ */
if (PageDirty(page) && mapping_cap_account_dirty(mapping)) { if (WARN_ON_ONCE(PageDirty(page)))
dec_zone_page_state(page, NR_FILE_DIRTY); account_page_cleaned(page, mapping);
dec_bdi_stat(inode_to_bdi(mapping->host), BDI_RECLAIMABLE);
}
} }
/** /**
......
...@@ -2110,6 +2110,25 @@ void account_page_dirtied(struct page *page, struct address_space *mapping) ...@@ -2110,6 +2110,25 @@ void account_page_dirtied(struct page *page, struct address_space *mapping)
} }
EXPORT_SYMBOL(account_page_dirtied); EXPORT_SYMBOL(account_page_dirtied);
/*
* Helper function for deaccounting dirty page without writeback.
*
* Doing this should *normally* only ever be done when a page
* is truncated, and is not actually mapped anywhere at all. However,
* fs/buffer.c does this when it notices that somebody has cleaned
* out all the buffers on a page without actually doing it through
* the VM. Can you say "ext3 is horribly ugly"? Thought you could.
*/
void account_page_cleaned(struct page *page, struct address_space *mapping)
{
if (mapping_cap_account_dirty(mapping)) {
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(inode_to_bdi(mapping->host), BDI_RECLAIMABLE);
task_io_account_cancelled_write(PAGE_CACHE_SIZE);
}
}
EXPORT_SYMBOL(account_page_cleaned);
/* /*
* For address_spaces which do not use buffers. Just tag the page as dirty in * For address_spaces which do not use buffers. Just tag the page as dirty in
* its radix tree. * its radix tree.
......
...@@ -92,35 +92,6 @@ void do_invalidatepage(struct page *page, unsigned int offset, ...@@ -92,35 +92,6 @@ void do_invalidatepage(struct page *page, unsigned int offset,
(*invalidatepage)(page, offset, length); (*invalidatepage)(page, offset, length);
} }
/*
* This cancels just the dirty bit on the kernel page itself, it
* does NOT actually remove dirty bits on any mmap's that may be
* around. It also leaves the page tagged dirty, so any sync
* activity will still find it on the dirty lists, and in particular,
* clear_page_dirty_for_io() will still look at the dirty bits in
* the VM.
*
* Doing this should *normally* only ever be done when a page
* is truncated, and is not actually mapped anywhere at all. However,
* fs/buffer.c does this when it notices that somebody has cleaned
* out all the buffers on a page without actually doing it through
* the VM. Can you say "ext3 is horribly ugly"? Tought you could.
*/
void cancel_dirty_page(struct page *page, unsigned int account_size)
{
if (TestClearPageDirty(page)) {
struct address_space *mapping = page->mapping;
if (mapping && mapping_cap_account_dirty(mapping)) {
dec_zone_page_state(page, NR_FILE_DIRTY);
dec_bdi_stat(inode_to_bdi(mapping->host),
BDI_RECLAIMABLE);
if (account_size)
task_io_account_cancelled_write(account_size);
}
}
}
EXPORT_SYMBOL(cancel_dirty_page);
/* /*
* If truncate cannot remove the fs-private metadata from the page, the page * If truncate cannot remove the fs-private metadata from the page, the page
* becomes orphaned. It will be left on the LRU and may even be mapped into * becomes orphaned. It will be left on the LRU and may even be mapped into
...@@ -140,7 +111,13 @@ truncate_complete_page(struct address_space *mapping, struct page *page) ...@@ -140,7 +111,13 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
if (page_has_private(page)) if (page_has_private(page))
do_invalidatepage(page, 0, PAGE_CACHE_SIZE); do_invalidatepage(page, 0, PAGE_CACHE_SIZE);
cancel_dirty_page(page, PAGE_CACHE_SIZE); /*
* Some filesystems seem to re-dirty the page even after
* the VM has canceled the dirty bit (eg ext3 journaling).
* Hence dirty accounting check is placed after invalidation.
*/
if (TestClearPageDirty(page))
account_page_cleaned(page, mapping);
ClearPageMappedToDisk(page); ClearPageMappedToDisk(page);
delete_from_page_cache(page); delete_from_page_cache(page);
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册