lkml.org 
[lkml]   [2009]   [Jan]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [BUG] mlocked page counter mismatch
From
Hi

> I think I see it. In try_to_unmap_anon(), called from try_to_munlock(),
> we have:
>
> list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
> if (MLOCK_PAGES && unlikely(unlock)) {
> if (!((vma->vm_flags & VM_LOCKED) &&
> !!! should be '||' ? ^^
> page_mapped_in_vma(page, vma)))
> continue; /* must visit all unlocked vmas */
> ret = SWAP_MLOCK; /* saw at least one mlocked vma */
> } else {
> ret = try_to_unmap_one(page, vma, migration);
> if (ret == SWAP_FAIL || !page_mapped(page))
> break;
> }
> if (ret == SWAP_MLOCK) {
> mlocked = try_to_mlock_page(page, vma);
> if (mlocked)
> break; /* stop if actually mlocked page */
> }
> }
>
> or that clause [under if (MLOCK_PAGES && unlikely(unlock))]
> might be clearer as:
>
> if ((vma->vm_flags & VM_LOCKED) && page_mapped_in_vma(page, vma))
> ret = SWAP_MLOCK; /* saw at least one mlocked vma */
> else
> continue; /* must visit all unlocked vmas */
>
> Do you agree?

Hmmm.
I don't think so.

> if (!((vma->vm_flags & VM_LOCKED) &&
> page_mapped_in_vma(page, vma)))
> continue; /* must visit all unlocked vmas */

is already equivalent to

> if ((vma->vm_flags & VM_LOCKED) && page_mapped_in_vma(page, vma))
> ret = SWAP_MLOCK; /* saw at least one mlocked vma */
> else
> continue; /* must visit all unlocked vmas */


> And, I wonder if we need a similar check for
> page_mapped_in_vma(page, vma) up in try_to_unmap_one()?

because page_mapped_in_vma() can return 0 if vma is anon vma only.

In the other word,
struct adress_space (for file) gurantee that unrelated vma doesn't chained.
but struct anon_vma (for anon) doesn't gurantee that unrelated vma
doesn't chained.


\
 
 \ /
  Last update: 2009-01-29 13:37    [W:0.271 / U:0.944 seconds]
©2003-2014 Jasper Spaans. Advertise on this site