* [PATCH] Flush dcache before writing into page to avoid alias
@ 2010-01-21 5:07 anfei zhou
2010-01-25 13:33 ` anfei
0 siblings, 1 reply; 7+ messages in thread
From: anfei zhou @ 2010-01-21 5:07 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Andrew Morton, KOSAKI Motohiro, linux, Jamie Lokier
The cache alias problem will happen if the changes of user shared mapping
is not flushed before copying, then user and kernel mapping may be mapped
into two different cache line, it is impossible to guarantee the coherence
after iov_iter_copy_from_user_atomic. So the right steps should be:
flush_dcache_page(page);
kmap_atomic(page);
write to page;
kunmap_atomic(page);
flush_dcache_page(page);
More precisely, we might create two new APIs flush_dcache_user_page and
flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
Here is a snippet tested on omap2430 with VIPT cache, and I think it is
not ARM-specific:
int val = 0x11111111;
fd = open("abc", O_RDWR);
addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
*(addr+0) = 0x44444444;
tmp = *(addr+0);
*(addr+1) = 0x77777777;
write(fd, &val, sizeof(int));
close(fd);
The results are not always 0x11111111 0x77777777 at the beginning as expected.
Signed-off-by: Anfei <anfei.zhou@gmail.com>
---
fs/fuse/file.c | 3 +++
mm/filemap.c | 3 +++
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index c18913a..a9f5e13 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -828,6 +828,9 @@ static ssize_t fuse_fill_write_pages(struct fuse_req *req,
if (!page)
break;
+ if (mapping_writably_mapped(mapping))
+ flush_dcache_page(page);
+
pagefault_disable();
tmp = iov_iter_copy_from_user_atomic(page, ii, offset, bytes);
pagefault_enable();
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();
--
1.6.3.1
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-21 5:07 [PATCH] Flush dcache before writing into page to avoid alias anfei zhou
@ 2010-01-25 13:33 ` anfei
2010-01-25 19:58 ` Andrew Morton
0 siblings, 1 reply; 7+ messages in thread
From: anfei @ 2010-01-25 13:33 UTC (permalink / raw)
To: linux-mm, linux-kernel, akpm; +Cc: KOSAKI Motohiro, linux, Jamie Lokier
Hi Andrew,
On Thu, Jan 21, 2010 at 01:07:57PM +0800, anfei zhou wrote:
> The cache alias problem will happen if the changes of user shared mapping
> is not flushed before copying, then user and kernel mapping may be mapped
> into two different cache line, it is impossible to guarantee the coherence
> after iov_iter_copy_from_user_atomic. So the right steps should be:
> flush_dcache_page(page);
> kmap_atomic(page);
> write to page;
> kunmap_atomic(page);
> flush_dcache_page(page);
> More precisely, we might create two new APIs flush_dcache_user_page and
> flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
>
> Here is a snippet tested on omap2430 with VIPT cache, and I think it is
> not ARM-specific:
> int val = 0x11111111;
> fd = open("abc", O_RDWR);
> addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> *(addr+0) = 0x44444444;
> tmp = *(addr+0);
> *(addr+1) = 0x77777777;
> write(fd, &val, sizeof(int));
> close(fd);
> The results are not always 0x11111111 0x77777777 at the beginning as expected.
>
Is this a real bug or not necessary to support?
Thanks,
Anfei.
> Signed-off-by: Anfei <anfei.zhou@gmail.com>
> ---
> fs/fuse/file.c | 3 +++
> mm/filemap.c | 3 +++
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index c18913a..a9f5e13 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -828,6 +828,9 @@ static ssize_t fuse_fill_write_pages(struct fuse_req *req,
> if (!page)
> break;
>
> + if (mapping_writably_mapped(mapping))
> + flush_dcache_page(page);
> +
> pagefault_disable();
> tmp = iov_iter_copy_from_user_atomic(page, ii, offset, bytes);
> pagefault_enable();
> 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();
> --
> 1.6.3.1
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-25 13:33 ` anfei
@ 2010-01-25 19:58 ` Andrew Morton
2010-01-25 20:00 ` Russell King - ARM Linux
2010-01-26 1:01 ` anfei zhou
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2010-01-25 19:58 UTC (permalink / raw)
To: anfei
Cc: linux-mm, linux-kernel, KOSAKI Motohiro, linux, Jamie Lokier,
linux-arm-kernel
On Mon, 25 Jan 2010 21:33:08 +0800 anfei <anfei.zhou@gmail.com> wrote:
> Hi Andrew,
>
> On Thu, Jan 21, 2010 at 01:07:57PM +0800, anfei zhou wrote:
> > The cache alias problem will happen if the changes of user shared mapping
> > is not flushed before copying, then user and kernel mapping may be mapped
> > into two different cache line, it is impossible to guarantee the coherence
> > after iov_iter_copy_from_user_atomic. So the right steps should be:
> > flush_dcache_page(page);
> > kmap_atomic(page);
> > write to page;
> > kunmap_atomic(page);
> > flush_dcache_page(page);
> > More precisely, we might create two new APIs flush_dcache_user_page and
> > flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
> >
> > Here is a snippet tested on omap2430 with VIPT cache, and I think it is
> > not ARM-specific:
> > int val = 0x11111111;
> > fd = open("abc", O_RDWR);
> > addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> > *(addr+0) = 0x44444444;
> > tmp = *(addr+0);
> > *(addr+1) = 0x77777777;
> > write(fd, &val, sizeof(int));
> > close(fd);
> > The results are not always 0x11111111 0x77777777 at the beginning as expected.
> >
> Is this a real bug or not necessary to support?
Bug. If variable `addr' has type int* then the contents of that file
should be 0x11111111 0x77777777. You didn't tell us what the contents
were in the incorrect case, but I guess it doesn't matter.
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-25 19:58 ` Andrew Morton
@ 2010-01-25 20:00 ` Russell King - ARM Linux
2010-01-27 21:59 ` Sebastian Andrzej Siewior
2010-01-26 1:01 ` anfei zhou
1 sibling, 1 reply; 7+ messages in thread
From: Russell King - ARM Linux @ 2010-01-25 20:00 UTC (permalink / raw)
To: Andrew Morton
Cc: anfei, linux-mm, linux-kernel, KOSAKI Motohiro, Jamie Lokier,
linux-arm-kernel
On Mon, Jan 25, 2010 at 11:58:14AM -0800, Andrew Morton wrote:
> On Mon, 25 Jan 2010 21:33:08 +0800 anfei <anfei.zhou@gmail.com> wrote:
>
> > Hi Andrew,
> >
> > On Thu, Jan 21, 2010 at 01:07:57PM +0800, anfei zhou wrote:
> > > The cache alias problem will happen if the changes of user shared mapping
> > > is not flushed before copying, then user and kernel mapping may be mapped
> > > into two different cache line, it is impossible to guarantee the coherence
> > > after iov_iter_copy_from_user_atomic. So the right steps should be:
> > > flush_dcache_page(page);
> > > kmap_atomic(page);
> > > write to page;
> > > kunmap_atomic(page);
> > > flush_dcache_page(page);
> > > More precisely, we might create two new APIs flush_dcache_user_page and
> > > flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
> > >
> > > Here is a snippet tested on omap2430 with VIPT cache, and I think it is
> > > not ARM-specific:
> > > int val = 0x11111111;
> > > fd = open("abc", O_RDWR);
> > > addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> > > *(addr+0) = 0x44444444;
> > > tmp = *(addr+0);
> > > *(addr+1) = 0x77777777;
> > > write(fd, &val, sizeof(int));
> > > close(fd);
> > > The results are not always 0x11111111 0x77777777 at the beginning as expected.
> > >
> > Is this a real bug or not necessary to support?
>
> Bug. If variable `addr' has type int* then the contents of that file
> should be 0x11111111 0x77777777. You didn't tell us what the contents
> were in the incorrect case, but I guess it doesn't matter.
FYI, from a previous email from anfei:
0x44444444 0x77777777
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-25 19:58 ` Andrew Morton
2010-01-25 20:00 ` Russell King - ARM Linux
@ 2010-01-26 1:01 ` anfei zhou
1 sibling, 0 replies; 7+ messages in thread
From: anfei zhou @ 2010-01-26 1:01 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, KOSAKI Motohiro, linux, Jamie Lokier,
linux-arm-kernel
On Tue, Jan 26, 2010 at 3:58 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Mon, 25 Jan 2010 21:33:08 +0800 anfei <anfei.zhou@gmail.com> wrote:
>
>> Hi Andrew,
>>
>> On Thu, Jan 21, 2010 at 01:07:57PM +0800, anfei zhou wrote:
>> > The cache alias problem will happen if the changes of user shared mapping
>> > is not flushed before copying, then user and kernel mapping may be mapped
>> > into two different cache line, it is impossible to guarantee the coherence
>> > after iov_iter_copy_from_user_atomic. So the right steps should be:
>> > flush_dcache_page(page);
>> > kmap_atomic(page);
>> > write to page;
>> > kunmap_atomic(page);
>> > flush_dcache_page(page);
>> > More precisely, we might create two new APIs flush_dcache_user_page and
>> > flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
>> >
>> > Here is a snippet tested on omap2430 with VIPT cache, and I think it is
>> > not ARM-specific:
>> > int val = 0x11111111;
>> > fd = open("abc", O_RDWR);
>> > addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
>> > *(addr+0) = 0x44444444;
>> > tmp = *(addr+0);
>> > *(addr+1) = 0x77777777;
>> > write(fd, &val, sizeof(int));
>> > close(fd);
>> > The results are not always 0x11111111 0x77777777 at the beginning as expected.
>> >
>> Is this a real bug or not necessary to support?
>
> Bug. If variable `addr' has type int* then the contents of that file
> should be 0x11111111 0x77777777. You didn't tell us what the contents
> were in the incorrect case, but I guess it doesn't matter.
>
Sorry, I didn't give the details, here is the old thread with more details:
http://linux.derkeiler.com/Mailing-Lists/Kernel/2010-01/msg07124.html
Regards,
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-25 20:00 ` Russell King - ARM Linux
@ 2010-01-27 21:59 ` Sebastian Andrzej Siewior
2010-01-27 22:12 ` Andrew Morton
0 siblings, 1 reply; 7+ messages in thread
From: Sebastian Andrzej Siewior @ 2010-01-27 21:59 UTC (permalink / raw)
To: Andrew Morton
Cc: Russell King - ARM Linux, anfei, linux-mm, linux-kernel,
KOSAKI Motohiro, Jamie Lokier, linux-arm-kernel
* Russell King - ARM Linux | 2010-01-25 20:00:04 [+0000]:
>On Mon, Jan 25, 2010 at 11:58:14AM -0800, Andrew Morton wrote:
>> On Mon, 25 Jan 2010 21:33:08 +0800 anfei <anfei.zhou@gmail.com> wrote:
>>
>> > Hi Andrew,
>> >
>> > On Thu, Jan 21, 2010 at 01:07:57PM +0800, anfei zhou wrote:
>> > > The cache alias problem will happen if the changes of user shared mapping
>> > > is not flushed before copying, then user and kernel mapping may be mapped
>> > > into two different cache line, it is impossible to guarantee the coherence
>> > > after iov_iter_copy_from_user_atomic. So the right steps should be:
>> > > flush_dcache_page(page);
>> > > kmap_atomic(page);
>> > > write to page;
>> > > kunmap_atomic(page);
>> > > flush_dcache_page(page);
>> > > More precisely, we might create two new APIs flush_dcache_user_page and
>> > > flush_dcache_kern_page to replace the two flush_dcache_page accordingly.
>> > >
>> > > Here is a snippet tested on omap2430 with VIPT cache, and I think it is
>> > > not ARM-specific:
>> > > int val = 0x11111111;
>> > > fd = open("abc", O_RDWR);
>> > > addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
>> > > *(addr+0) = 0x44444444;
>> > > tmp = *(addr+0);
>> > > *(addr+1) = 0x77777777;
>> > > write(fd, &val, sizeof(int));
>> > > close(fd);
>> > > The results are not always 0x11111111 0x77777777 at the beginning as expected.
>> > >
>> > Is this a real bug or not necessary to support?
>>
>> Bug. If variable `addr' has type int* then the contents of that file
>> should be 0x11111111 0x77777777. You didn't tell us what the contents
>> were in the incorrect case, but I guess it doesn't matter.
>
>FYI, from a previous email from anfei:
>
>0x44444444 0x77777777
I just wanted to query what the status of this patch is. This patch
seems to fix a real bug which causes a test suite to fail on ARM [0].
The test suite passes on my VIVT ARM with this patch.
[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524003
Sebastian
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Flush dcache before writing into page to avoid alias
2010-01-27 21:59 ` Sebastian Andrzej Siewior
@ 2010-01-27 22:12 ` Andrew Morton
0 siblings, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2010-01-27 22:12 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Russell King - ARM Linux, anfei, linux-mm, linux-kernel,
KOSAKI Motohiro, Jamie Lokier, linux-arm-kernel
On Wed, 27 Jan 2010 22:59:59 +0100
Sebastian Andrzej Siewior <sebastian@breakpoint.cc> wrote:
> I just wanted to query what the status of this patch is. This patch
> seems to fix a real bug which causes a test suite to fail on ARM [0].
> The test suite passes on my VIVT ARM with this patch.
>
> [0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=524003
I have it queued for 2.6.33, backportable to 2.6.32.x, assuming that
nobody sees any issues with it.
--
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: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-01-27 22:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-21 5:07 [PATCH] Flush dcache before writing into page to avoid alias anfei zhou
2010-01-25 13:33 ` anfei
2010-01-25 19:58 ` Andrew Morton
2010-01-25 20:00 ` Russell King - ARM Linux
2010-01-27 21:59 ` Sebastian Andrzej Siewior
2010-01-27 22:12 ` Andrew Morton
2010-01-26 1:01 ` anfei zhou
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).