Messages in this thread Patch in this message | | | From | KOSAKI Motohiro <> | Subject | Re: [PATCH] [for mmotm-1113] mm: Simplify try_to_unmap_one() | Date | Thu, 19 Nov 2009 10:23:16 +0900 (JST) |
| |
> On Tue, 17 Nov 2009 17:39:27 +0900 (JST) > KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote: > > > +out_mlock: > > + pte_unmap_unlock(pte, ptl); > > + > > + if (down_read_trylock(&vma->vm_mm->mmap_sem)) { > > + if (vma->vm_flags & VM_LOCKED) { > > + mlock_vma_page(page); > > + ret = SWAP_MLOCK; > > } > > + up_read(&vma->vm_mm->mmap_sem); > > It's somewhat unobvious why we're using a trylock here. Ranking versus > lock_page(), perhaps? > > In general I think a trylock should have an associated comment which explains > > a) why it is being used at this site and > > b) what happens when the trylock fails - why this isn't a > bug, how the kernel recovers from the inconsistency, what its > overall effect is, etc. > > <wonders why we need to take mmap_sem here at all>
This mmap_sem is needed certainenaly. Following comment is sufficient?
--- mm/rmap.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/mm/rmap.c b/mm/rmap.c index 70dec01..b1c9342 100644 --- a/mm/rmap.c +++ b/mm/rmap.c @@ -860,6 +860,14 @@ out: out_mlock: pte_unmap_unlock(pte, ptl); + + /* + * We need mmap_sem locking, Otherwise VM_LOCKED check makes + * unstable result and race. Plus, We can't wait here because + * we now hold anon_vma->lock or mapping->i_mmap_lock. + * If trylock failed, The page remain evictable lru and + * retry to more unevictable lru by later vmscan. + */ if (down_read_trylock(&vma->vm_mm->mmap_sem)) { if (vma->vm_flags & VM_LOCKED) { mlock_vma_page(page); -- 1.6.2.5
| |