Re: Flushing whole page instead of work for ptrace

[Minchan Kim]

On Fri, Dec 03, 2010 at 04:00:21PM +0100, Oleg Nesterov wrote:
> On 11/30, Roland McGrath wrote:
> >
> > Documentation/cachetlb.txt says:
> >
> > 	Any time the kernel writes to a page cache page, _OR_
> > 	the kernel is about to read from a page cache page and
> > 	user space shared/writable mappings of this page potentially
> > 	exist, this routine is called.
> >
> > In your case, the kernel is only reading (write=0 passed to
> > access_process_vm and get_user_pages).  In normal situations,
> > the page in question will have only a private and read-only
> > mapping in user space.  So the call should not be required in
> > these cases--if the code can tell that's so.
> >
> > Perhaps something like the following would be safe.
> > But you really need some VM folks to tell you for sure.
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 02e48aa..2864ee7 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -1484,7 +1484,8 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
> >  				pages[i] = page;
> >
> >  				flush_anon_page(vma, page, start);
> > -				flush_dcache_page(page);
> > +				if ((vm_flags & VM_WRITE) || (vma->vm_flags & VM_SHARED)
> > +					flush_dcache_page(page);
> 
> First of all, I know absolutely nothing about D-cache aliasing.
> My poor understanding of flush_dcache_page() is: synchronize the
> kernel/user vision of this memory, in the case when either side
> can change it.
> 
> If this is true, then this change doesn't look right in general.
> 
> Even if (vma->vm_flags & VM_SHARED) == 0, it is possible that
> tsk can write to this memory, this mapping can be writable and
> private.
> 
> Even if we ensure that this mapping is readonly/private, another
> user-space process can write to this page via shared/writable
> mapping.
> 

I think you're right. It has a portential that other processes have
a such mapping.

> 
> I'd like to know if my understanding is correct, I am just curious.
> 
> Oleg.

How about this?
Maybe this patch would mitigate the overhead.
But I am not sure this patch. Cced GUP experts.