Messages in this thread | | | Date | Tue, 3 Feb 2009 08:27:19 +0900 | From | MinChan Kim <> | Subject | Re: [PATCH] fix mlocked page counter mismatch |
| |
On Mon, Feb 02, 2009 at 12:16:35PM -0500, Lee Schermerhorn wrote: > On Mon, 2009-02-02 at 15:16 +0900, MinChan Kim wrote: > > When I tested following program, I found that mlocked counter > > is strange. > > It couldn't free some mlocked pages of test program. > > > > It is caused that try_to_unmap_file don't check real > > page mapping in vmas. > > That's because goal of address_space for file is to find all processes > > into which the file's specific interval is mapped. > > What I mean is that it's not related page but file's interval. > > > > Even if the page isn't really mapping at the vma, it returns > > SWAP_MLOCK since the vma have VM_LOCKED, then calls > > try_to_mlock_page. After all, mlocked counter is increased again. > > > > This patch is based on 2.6.28-rc2-mm1. > > > > -- my test program -- > > > > #include <stdio.h> > > #include <sys/mman.h> > > int main() > > { > > mlockall(MCL_CURRENT); > > return 0; > > } > > > > -- before -- > > > > root@barrios-target-linux:~# cat /proc/meminfo | egrep 'Mlo|Unev' > > Unevictable: 0 kB > > Mlocked: 0 kB > > > > -- after -- > > > > root@barrios-target-linux:~# cat /proc/meminfo | egrep 'Mlo|Unev' > > Unevictable: 8 kB > > Mlocked: 8 kB > > > > > > -- > > > > diff --git a/mm/rmap.c b/mm/rmap.c > > index 1099394..9ba1fdf 100644 > > --- a/mm/rmap.c > > +++ b/mm/rmap.c > > @@ -1073,6 +1073,9 @@ static int try_to_unmap_file(struct page *page, int unlock, int migration) > > unsigned long max_nl_size = 0; > > unsigned int mapcount; > > unsigned int mlocked = 0; > > + unsigned long address; > > + pte_t *pte; > > + spinlock_t *ptl; > > > > if (MLOCK_PAGES && unlikely(unlock)) > > ret = SWAP_SUCCESS; /* default for try_to_munlock() */ > > @@ -1089,6 +1092,13 @@ static int try_to_unmap_file(struct page *page, int unlock, int migration) > > goto out; > > } > > if (ret == SWAP_MLOCK) { > > + address = vma_address(page, vma); > > + if (address != -EFAULT) { > > + pte = page_check_address(page, vma->vm_mm, address, &ptl, 0); > > + if (!pte) > > + continue; > > + pte_unmap_unlock(pte, ptl); > > + } > > mlocked = try_to_mlock_page(page, vma); > > if (mlocked) > > break; /* stop if actually mlocked page */ > > Hi, MinChan: > > Interestingly, Rik had addressed this [simpler patch below] way back > when he added the page_mapped_in_vma() function. I asked him whether
> the rb tree shouldn't have filtered any vmas that didn't have the page > mapped. He agreed and removed the check from try_to_unmap_file(). > Guess I can be very convincing, even when I'm wrong [happening a lot > lately]. Of course, in this instance, the rb-tree filtering only works > for shared, page-cache pages. The problem uncovered by your test case
It's not rb-tree but priority tree. ;-)
> is with a COWed anon page in a file-backed vma. Yes, the vma 'maps' the > virtual address range containing the page in question, but since it's a > private COWed anon page, it isn't necessarily "mapped" in the VM_LOCKED > vma's mm's page table. We need the check...
Indeed!
> > I've added the variant below [CURRENTLY UNTESTED] to my test tree. > > Lee > > [intentionally omitted sign off, until tested.]
I shouldn't forgot page_mapped_in_vma. However, It looks good to me. Thank you for testing.
> > > Index: linux-2.6.29-rc3/mm/rmap.c > =================================================================== > --- linux-2.6.29-rc3.orig/mm/rmap.c 2009-01-30 14:13:56.000000000 -0500 > +++ linux-2.6.29-rc3/mm/rmap.c 2009-02-02 11:27:11.000000000 -0500 > @@ -1072,7 +1072,8 @@ static int try_to_unmap_file(struct page > spin_lock(&mapping->i_mmap_lock); > vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) { > if (MLOCK_PAGES && unlikely(unlock)) { > - if (!(vma->vm_flags & VM_LOCKED)) > + if (!((vma->vm_flags & VM_LOCKED) && > + page_mapped_in_vma(page, vma))) > continue; /* must visit all vmas */ > ret = SWAP_MLOCK; > } else { >
-- Kinds Regards MinChan Kim
| |