linux-kernel - Re: [PATCH 1/1] pagemap clear_refs: modify to specify anon or mapped vma clearing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7928e7bd0907241100n5bcef0d8nff0e769f19070ebb@mail.gmail.com>
Date:	Fri, 24 Jul 2009 11:00:46 -0700
From:	Moussa Ba <moussa.a.ba@...il.com>
To:	Amerigo Wang <xiyou.wangcong@...il.com>
Cc:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	adobriyan@...il.com, mpm@...enic.com, mel@....ul.ie,
	yinghan@...gle.com, npiggin@...e.de, jaredeh@...il.com
Subject: Re: [PATCH 1/1] pagemap clear_refs: modify to specify anon or mapped 
	vma clearing

On Fri, Jul 24, 2009 at 1:39 AM, Amerigo Wang <xiyou.wangcong@...il.com> wrote:
>
> On Thu, Jul 23, 2009 at 08:57:22PM -0700, Moussa A. Ba wrote:
> >
> > Signed-off-by: Jared E. Hulbert <jaredeh@...il.com>
> > Signed-off-by: Moussa Ba <moussa.a.ba@...il.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@...r.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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/