From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail138.messagelabs.com (mail138.messagelabs.com [216.82.249.35]) by kanga.kvack.org (Postfix) with SMTP id 8F5746B006A for ; Wed, 20 Jan 2010 23:59:59 -0500 (EST) Received: by pwj10 with SMTP id 10so4481453pwj.6 for ; Wed, 20 Jan 2010 20:59:57 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20100121094733.3778.A69D9226@jp.fujitsu.com> References: <20100120174630.4071.A69D9226@jp.fujitsu.com> <20100120095242.GA5672@desktop> <20100121094733.3778.A69D9226@jp.fujitsu.com> Date: Thu, 21 Jan 2010 12:59:57 +0800 Message-ID: <979dd0561001202059v1870870oaa4df876aa12a756@mail.gmail.com> Subject: Re: cache alias in mmap + write From: anfei zhou Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Sender: owner-linux-mm@kvack.org To: KOSAKI Motohiro Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux@arm.linux.org.uk, jamie@shareable.org List-ID: On Thu, Jan 21, 2010 at 9:10 AM, KOSAKI Motohiro wrote: >> 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: >> > > =A0 =A0 =A0 =A0 =A0 if (unlikely(status)) >> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; >> > > >> > > + =A0 =A0 =A0 =A0 if (mapping_writably_mapped(mapping)) >> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flush_dcache_page(page); >> > > + >> > > =A0 =A0 =A0 =A0 =A0 pagefault_disable(); >> > > =A0 =A0 =A0 =A0 =A0 copied =3D iov_iter_copy_from_user_atomic(page, = i, offset, bytes); >> > > =A0 =A0 =A0 =A0 =A0 pagefault_enable(); >> > >> > I'm not sure ARM cache coherency model. but I guess correct patch is h= ere. >> > >> > + =A0 =A0 =A0 =A0 =A0 if (mapping_writably_mapped(mapping)) >> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 flush_dcache_page(page); >> > + >> > =A0 =A0 =A0 =A0 =A0 =A0 pagefault_disable(); >> > =A0 =A0 =A0 =A0 =A0 =A0 copied =3D iov_iter_copy_from_user_atomic(page= , i, offset, bytes); >> > =A0 =A0 =A0 =A0 =A0 =A0 pagefault_enable(); >> > - =A0 =A0 =A0 =A0 =A0 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, =A0and the one I added is used to flush the userspace changes. >> And I think it's better to split this function into two: >> =A0 =A0 =A0 flush_dcache_user_page(page); >> =A0 =A0 =A0 kmap_atomic(page); >> =A0 =A0 =A0 write to =A0page; >> =A0 =A0 =A0 kunmap_atomic(page); >> =A0 =A0 =A0 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. > I will resend the patch, if this patch is acceptable, I will create another patch 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. > =A01. Move flush_dcache_page() into iov_iter_copy_from_user_atomic(). > =A0 =A0Your above explanation indicate it is real intention. plus, change > =A0 =A0iov_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 plac= es, So I just add flush_dcache_page directly. > =A02. Add some commnet. almost developer only have x86 machine. so, arm > =A0 =A0specific trick need additional explicit explanation. otherwise any= body > =A0 =A0might break this code in the future. > =A03. Resend the patch. original mail isn't good patch format. please con= sider > =A0 =A0to reduce akpm suffer. > I will send it soon. Thanks, Anfei. > > > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org