Re: [BUG] mlocked page counter mismatch

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

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.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>