Skip to content

Commit

Permalink
page_writeback: revive cancel_dirty_page() in a restricted form
Browse files Browse the repository at this point in the history
cancel_dirty_page() had some issues and b9ea251 ("page_writeback:
clean up mess around cancel_dirty_page()") replaced it with
account_page_cleaned() which makes the caller responsible for clearing
the dirty bit; unfortunately, the planned changes for cgroup writeback
support requires synchronization between dirty bit manipulation and
stat updates.  While we can open-code such synchronization in each
account_page_cleaned() callsite, that's gonna be unnecessarily awkward
and verbose.

This patch revives cancel_dirty_page() but in a more restricted form.
All it does is TestClearPageDirty() followed by account_page_cleaned()
invocation if the page was dirty.  This helper covers all
account_page_cleaned() usages except for __delete_from_page_cache()
which is a special case anyway and left alone.  As this leaves no
module user for account_page_cleaned(), EXPORT_SYMBOL() is dropped
from it.

This patch just revives cancel_dirty_page() as a trivial wrapper to
replace equivalent usages and doesn't introduce any functional
changes.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Konstantin Khlebnikov <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
htejun authored and axboe committed Jun 2, 2015
1 parent f26cdc8 commit 11f81be
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
if (PagePrivate(page))
page->mapping->a_ops->invalidatepage(page, 0, PAGE_CACHE_SIZE);

if (TestClearPageDirty(page))
account_page_cleaned(page, mapping);

cancel_dirty_page(page);
ClearPageMappedToDisk(page);
ll_delete_from_page_cache(page);
}
Expand Down
4 changes: 2 additions & 2 deletions fs/buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -3232,8 +3232,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 && TestClearPageDirty(page))
account_page_cleaned(page, mapping);
if (ret)
cancel_dirty_page(page);
spin_unlock(&mapping->private_lock);
out:
if (buffers_to_free) {
Expand Down
1 change: 1 addition & 0 deletions include/linux/mm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,7 @@ 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);
void cancel_dirty_page(struct page *page);
int clear_page_dirty_for_io(struct page *page);

int get_cmdline(struct task_struct *task, char *buffer, int buflen);
Expand Down
27 changes: 20 additions & 7 deletions mm/page-writeback.c
Original file line number Diff line number Diff line change
Expand Up @@ -2112,12 +2112,6 @@ 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)
{
Expand All @@ -2127,7 +2121,6 @@ void account_page_cleaned(struct page *page, struct address_space *mapping)
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
Expand Down Expand Up @@ -2265,6 +2258,26 @@ int set_page_dirty_lock(struct page *page)
}
EXPORT_SYMBOL(set_page_dirty_lock);

/*
* 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"? Thought you could.
*/
void cancel_dirty_page(struct page *page)
{
if (TestClearPageDirty(page))
account_page_cleaned(page, page_mapping(page));
}
EXPORT_SYMBOL(cancel_dirty_page);

/*
* Clear a page's dirty flag, while caring for dirty memory accounting.
* Returns true if the page was previously dirty.
Expand Down
4 changes: 1 addition & 3 deletions mm/truncate.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ truncate_complete_page(struct address_space *mapping, struct page *page)
* 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);

cancel_dirty_page(page);
ClearPageMappedToDisk(page);
delete_from_page_cache(page);
return 0;
Expand Down

0 comments on commit 11f81be

Please sign in to comment.