Re: cache alias in mmap + write

[KOSAKI Motohiro]

> On Wed, Jan 20, 2010 at 06:10:11PM +0900, KOSAKI Motohiro wrote:
> > Hello,
> > 
> > > diff --git a/mm/filemap.c b/mm/filemap.c
> > > index 96ac6b0..07056fb 100644
> > > --- a/mm/filemap.c
> > > +++ b/mm/filemap.c
> > > @@ -2196,6 +2196,9 @@ again:
> > >  		if (unlikely(status))
> > >  			break;
> > >  
> > > +		if (mapping_writably_mapped(mapping))
> > > +			flush_dcache_page(page);
> > > +
> > >  		pagefault_disable();
> > >  		copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes);
> > >  		pagefault_enable();
> > 
> > I'm not sure ARM cache coherency model. but I guess correct patch is here.
> > 
> > +		if (mapping_writably_mapped(mapping))
> > +			flush_dcache_page(page);
> > +
> >  		pagefault_disable();
> >  		copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes);
> >  		pagefault_enable();
> > -		flush_dcache_page(page);
> > 
> > Why do we need to call flush_dcache_page() twice?
> > 
> The latter flush_dcache_page is used to flush the kernel changes
> (iov_iter_copy_from_user_atomic), which makes the userspace to see the
> write,  and the one I added is used to flush the userspace changes.
> And I think it's better to split this function into two:
> 	flush_dcache_user_page(page);
> 	kmap_atomic(page);
> 	write to  page;
> 	kunmap_atomic(page);
> 	flush_dcache_kern_page(page);
> But currently there is no such API.

Why can't we create new api? this your pseudo code looks very fine to me.


note: if you don't like to create new api. I can agree your current patch.
but I have three requests.
 1. Move flush_dcache_page() into iov_iter_copy_from_user_atomic().
    Your above explanation indicate it is real intention. plus, change
    iov_iter_copy_from_user_atomic() fixes fuse too.
 2. Add some commnet. almost developer only have x86 machine. so, arm
    specific trick need additional explicit explanation. otherwise anybody
    might break this code in the future.
 3. Resend the patch. original mail isn't good patch format. please consider
    to reduce akpm suffer.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/