Skip to content

Commit

Permalink
mm: fix clear_page_dirty_for_io vs fault race
Browse files Browse the repository at this point in the history
Fix msync data loss and (less importantly) dirty page accounting
inaccuracies due to the race remaining in clear_page_dirty_for_io().

The deleted comment explains what the race was, and the added comments
explain how it is fixed.

Signed-off-by: Nick Piggin <npiggin@suse.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Nick Piggin authored and Linus Torvalds committed Jul 19, 2007
1 parent 83c5407 commit 7935289
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
9 changes: 9 additions & 0 deletions mm/memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -1765,6 +1765,15 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
unlock:
pte_unmap_unlock(page_table, ptl);
if (dirty_page) {
/*
* Yes, Virginia, this is actually required to prevent a race
* with clear_page_dirty_for_io() from clearing the page dirty
* bit after it clear all dirty ptes, but before a racing
* do_wp_page installs a dirty pte.
*
* do_no_page is protected similarly.
*/
wait_on_page_locked(dirty_page);
set_page_dirty_balance(dirty_page);
put_page(dirty_page);
}
Expand Down
17 changes: 12 additions & 5 deletions mm/page-writeback.c
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,8 @@ int clear_page_dirty_for_io(struct page *page)
{
struct address_space *mapping = page_mapping(page);

BUG_ON(!PageLocked(page));

if (mapping && mapping_cap_account_dirty(mapping)) {
/*
* Yes, Virginia, this is indeed insane.
Expand All @@ -943,14 +945,19 @@ int clear_page_dirty_for_io(struct page *page)
* We basically use the page "master dirty bit"
* as a serialization point for all the different
* threads doing their things.
*
* FIXME! We still have a race here: if somebody
* adds the page back to the page tables in
* between the "page_mkclean()" and the "TestClearPageDirty()",
* we might have it mapped without the dirty bit set.
*/
if (page_mkclean(page))
set_page_dirty(page);
/*
* We carefully synchronise fault handlers against
* installing a dirty pte and marking the page dirty
* at this point. We do this by having them hold the
* page lock at some point after installing their
* pte, but before marking the page dirty.
* Pages are always locked coming in here, so we get
* the desired exclusion. See mm/memory.c:do_wp_page()
* for more comments.
*/
if (TestClearPageDirty(page)) {
dec_zone_page_state(page, NR_FILE_DIRTY);
return 1;
Expand Down

0 comments on commit 7935289

Please sign in to comment.