Re: cache alias in mmap + write
From: anfei zhou
Date: Thu Jan 21 2010 - 00:00:14 EST
On Thu, Jan 21, 2010 at 9:10 AM, 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:
>> write to page;
>> But currently there is no such API.
> Why can't we create new api? this your pseudo code looks very fine to me.
I will resend the patch, if this patch is acceptable, I will create
to introduce this new API.
> 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.
There is a check on mapping, that's not passed into
iov_iter_copy_from_user_atomic, and this function is only called a few places,
So I just add flush_dcache_page directly.
> 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.
I will send it soon.
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/