Messages in this thread Patch in this message | | | Date | Fri, 24 Jul 2009 11:00:46 -0700 | Subject | Re: [PATCH 1/1] pagemap clear_refs: modify to specify anon or mapped vma clearing | From | Moussa Ba <> |
| |
On Fri, Jul 24, 2009 at 1:39 AM, Amerigo Wang <xiyou.wangcong@gmail.com> wrote: > > On Thu, Jul 23, 2009 at 08:57:22PM -0700, Moussa A. Ba wrote: > > > > Signed-off-by: Jared E. Hulbert <jaredeh@gmail.com> > > Signed-off-by: Moussa Ba <moussa.a.ba@gmail.com> > > --- a/fs/proc/task_mmu.c 2009-07-21 14:30:01.000000000 -0700 > > +++ b/fs/proc/task_mmu.c 2009-07-21 14:32:56.000000000 -0700 > > @@ -461,6 +461,33 @@ > > cond_resched(); > > return 0; > > } > > +static void walk_vma_area(struct mm_walk *this_walk, struct > > vm_area_struct *vma, > > + int type) > > +{ > > + switch (type) { > > + /* Clear Anon VMAs only */ > > + case 1: > > + if (!is_vm_hugetlb_page(vma) && !vma->vm_file) > > + walk_page_range(vma->vm_start, > > + vma->vm_end, > > + this_walk); > > + break; > > + /* Clear Mapped VMAs only */ > > + case 2: > > + if (!is_vm_hugetlb_page(vma) && vma->vm_file) > > + walk_page_range(vma->vm_start, > > + vma->vm_end, > > + this_walk); > > + break; > > + /* Clear All VMAs */ > > + default: > > + if (!is_vm_hugetlb_page(vma)) > > + walk_page_range(vma->vm_start, > > + vma->vm_end, > > + this_walk); > > + break; > > + } > > +} > > > > > First, can you make these plain numbers as macros or enum types? > > Second, I believe the above code can be simiplified, how about below?
> > { > if (!is_vm_hugetlb_page(vma)) { > if (type == 1 && vma->vm_file) > return; > if (type == 2 && !vma->vm_file) > return; > walk_page_range(vma->vm_start, vma->end, this_walk); > } > } >
It would indeed work, it can probably be written as:
{ if (is_vm_hugetlb_page(vma)) return; if (type == 2 && vma->vm_file) return; if (type == 3 && !vma->vm_file) return; walk_page_range(vma->vm_start, vma->vm_end, this_walk);
}
The change in numbers also addresses Matt's concern that writing a 1 is the default behavior where all vmas are cleared, hence it is better to not break that and make the selective clearing options 2 and 3. There is no documentation for clear_refs beyond a 1 liner that states "Clears page referenced bits shown in smaps output". I will also update the Documentation and add it to the patch.
> > > > static ssize_t clear_refs_write(struct file *file, const char __user * > > buf, > > size_t count, loff_t * ppos) > > @@ -469,13 +496,15 @@ > > char buffer[PROC_NUMBUF], *end; > > struct mm_struct *mm; > > struct vm_area_struct *vma; > > + int type; > > > > memset(buffer, 0, sizeof(buffer)); > > if (count > sizeof(buffer) - 1) > > count = sizeof(buffer) - 1; > > if (copy_from_user(buffer, buf, count)) > > return -EFAULT; > > - if (!simple_strtol(buffer, &end, 0)) > > + type = simple_strtol(buffer, &end, 0); > > + if (!type) > > return -EINVAL; > > if (*end == '\n') > > end++; > > @@ -491,9 +520,7 @@ > > down_read(&mm->mmap_sem); > > for (vma = mm->mmap; vma; vma = vma->vm_next) { > > clear_refs_walk.private = vma; > > - if (!is_vm_hugetlb_page(vma)) > > - walk_page_range(vma->vm_start, vma->vm_end, > > - &clear_refs_walk); > > + walk_vma_area(&clear_refs_walk, vma, type); > > } > > flush_tlb_mm(mm); > > up_read(&mm->mmap_sem); > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |