Commit e15f8c01 authored by Michel Lespinasse's avatar Michel Lespinasse Committed by Linus Torvalds

mlock: fix race when munlocking pages in do_wp_page()

vmscan can lazily find pages that are mapped within VM_LOCKED vmas, and
set the PageMlocked bit on these pages, transfering them onto the
unevictable list.  When do_wp_page() breaks COW within a VM_LOCKED vma,
it may need to clear PageMlocked on the old page and set it on the new
page instead.

This change fixes an issue where do_wp_page() was clearing PageMlocked
on the old page while the pte was still pointing to it (as well as
rmap).  Therefore, we were not protected against vmscan immediately
transfering the old page back onto the unevictable list.  This could
cause pages to get stranded there forever.

I propose to move the corresponding code to the end of do_wp_page(),
after the pte (and rmap) have been pointed to the new page.
Additionally, we can use munlock_vma_page() instead of
clear_page_mlock(), so that the old page stays mlocked if there are
still other VM_LOCKED vmas mapping it.
Signed-off-by: default avatarMichel Lespinasse <walken@google.com>
Reviewed-by: default avatarKAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: default avatarHugh Dickins <hughd@google.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent e6d2e2b2
...@@ -2219,7 +2219,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -2219,7 +2219,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
&ptl); &ptl);
if (!pte_same(*page_table, orig_pte)) { if (!pte_same(*page_table, orig_pte)) {
unlock_page(old_page); unlock_page(old_page);
page_cache_release(old_page);
goto unlock; goto unlock;
} }
page_cache_release(old_page); page_cache_release(old_page);
...@@ -2289,7 +2288,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -2289,7 +2288,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
&ptl); &ptl);
if (!pte_same(*page_table, orig_pte)) { if (!pte_same(*page_table, orig_pte)) {
unlock_page(old_page); unlock_page(old_page);
page_cache_release(old_page);
goto unlock; goto unlock;
} }
...@@ -2367,16 +2365,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -2367,16 +2365,6 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
} }
__SetPageUptodate(new_page); __SetPageUptodate(new_page);
/*
* Don't let another task, with possibly unlocked vma,
* keep the mlocked page.
*/
if ((vma->vm_flags & VM_LOCKED) && old_page) {
lock_page(old_page); /* for LRU manipulation */
clear_page_mlock(old_page);
unlock_page(old_page);
}
if (mem_cgroup_newpage_charge(new_page, mm, GFP_KERNEL)) if (mem_cgroup_newpage_charge(new_page, mm, GFP_KERNEL))
goto oom_free_new; goto oom_free_new;
...@@ -2444,10 +2432,20 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma, ...@@ -2444,10 +2432,20 @@ static int do_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
if (new_page) if (new_page)
page_cache_release(new_page); page_cache_release(new_page);
if (old_page)
page_cache_release(old_page);
unlock: unlock:
pte_unmap_unlock(page_table, ptl); pte_unmap_unlock(page_table, ptl);
if (old_page) {
/*
* Don't let another task, with possibly unlocked vma,
* keep the mlocked page.
*/
if ((ret & VM_FAULT_WRITE) && (vma->vm_flags & VM_LOCKED)) {
lock_page(old_page); /* LRU manipulation */
munlock_vma_page(old_page);
unlock_page(old_page);
}
page_cache_release(old_page);
}
return ret; return ret;
oom_free_new: oom_free_new:
page_cache_release(new_page); page_cache_release(new_page);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment