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 commit2d6d7f9828
("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 commits46d2277c79
("Clean up and make try_to_free_buffers() not race with dirty pages") and3e67c0987d
("truncate: clear page dirtiness before running try_to_free_buffers()") first was reverted right in v2.6.20 in commitecdfc9787f
("Resurrect 'try_to_free_buffers()' VM hackery"), second in v2.6.25 commita2b345642f
("Fix dirty page accounting leak with ext3 data=journal"). Custom fixes were introduced between these points. NFS in v2.6.23, commit1b3b4a1a2d
("NFS: Fix a write request leak in nfs_invalidate_page()"). Kludge in __delete_from_page_cache() in v2.6.24, commit3a6927906f
("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: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Cc: Tejun Heo <tj@kernel.org> Cc: Jan Kara <jack@suse.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
ca0984caa8
commit
b9ea25152e
|
@ -55,7 +55,9 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
|
|||
if (PagePrivate(page))
|
||||
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);
|
||||
ll_delete_from_page_cache(page);
|
||||
}
|
||||
|
|
|
@ -3243,8 +3243,8 @@ int try_to_free_buffers(struct page *page)
|
|||
* to synchronise against __set_page_dirty_buffers and prevent the
|
||||
* dirty bit from being lost.
|
||||
*/
|
||||
if (ret)
|
||||
cancel_dirty_page(page, PAGE_CACHE_SIZE);
|
||||
if (ret && TestClearPageDirty(page))
|
||||
account_page_cleaned(page, mapping);
|
||||
spin_unlock(&mapping->private_lock);
|
||||
out:
|
||||
if (buffers_to_free) {
|
||||
|
|
|
@ -319,7 +319,7 @@ static int hugetlbfs_write_end(struct file *file, struct address_space *mapping,
|
|||
|
||||
static void truncate_huge_page(struct page *page)
|
||||
{
|
||||
cancel_dirty_page(page, /* No IO accounting for huge pages? */0);
|
||||
ClearPageDirty(page);
|
||||
ClearPageUptodate(page);
|
||||
delete_from_page_cache(page);
|
||||
}
|
||||
|
|
|
@ -1876,11 +1876,6 @@ int nfs_wb_page_cancel(struct inode *inode, struct page *page)
|
|||
* request from the inode / page_private pointer and
|
||||
* release it */
|
||||
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);
|
||||
}
|
||||
|
||||
|
|
|
@ -1294,9 +1294,11 @@ int __set_page_dirty_no_writeback(struct page *page);
|
|||
int redirty_page_for_writepage(struct writeback_control *wbc,
|
||||
struct page *page);
|
||||
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_lock(struct page *page);
|
||||
int clear_page_dirty_for_io(struct page *page);
|
||||
|
||||
int get_cmdline(struct task_struct *task, char *buffer, int buflen);
|
||||
|
||||
/* Is the vma a continuation of the stack vma above it? */
|
||||
|
|
|
@ -328,8 +328,6 @@ static inline void SetPageUptodate(struct page *page)
|
|||
|
||||
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_set_page_writeback(struct page *page, bool keep_write);
|
||||
|
||||
|
|
15
mm/filemap.c
15
mm/filemap.c
|
@ -203,16 +203,15 @@ void __delete_from_page_cache(struct page *page, void *shadow)
|
|||
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).
|
||||
* At this point page must be either written or cleaned by truncate.
|
||||
* Dirty page here signals a bug and loss of unwritten data.
|
||||
*
|
||||
* Fix it up by doing a final dirty accounting check after
|
||||
* having removed the page entirely.
|
||||
* This fixes dirty accounting after removing the page entirely but
|
||||
* 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)) {
|
||||
dec_zone_page_state(page, NR_FILE_DIRTY);
|
||||
dec_bdi_stat(inode_to_bdi(mapping->host), BDI_RECLAIMABLE);
|
||||
}
|
||||
if (WARN_ON_ONCE(PageDirty(page)))
|
||||
account_page_cleaned(page, mapping);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -2110,6 +2110,25 @@ void account_page_dirtied(struct page *page, struct address_space *mapping)
|
|||
}
|
||||
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
|
||||
* its radix tree.
|
||||
|
|
|
@ -92,35 +92,6 @@ void do_invalidatepage(struct page *page, unsigned int offset,
|
|||
(*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
|
||||
* 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)
|
|||
if (page_has_private(page))
|
||||
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);
|
||||
delete_from_page_cache(page);
|
||||
|
|
Loading…
Reference in New Issue