Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: Ralf Baechle <ralf@linux-mips.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-mips@linux-mips.org, Ravi.Pratap@hillcrestlabs.com
Subject: Re: flush_anon_page for MIPS
Date: Fri, 23 Mar 2007 14:19:39 +0000	[thread overview]
Message-ID: <20070323141939.GB17311@linux-mips.org> (raw)
In-Reply-To: <E1HUVlw-00034H-00@dorka.pomaz.szeredi.hu>

On Thu, Mar 22, 2007 at 11:28:40PM +0100, Miklos Szeredi wrote:

> It seems that MIPS needs to implement flush_anon_page() for correct
> operation of get_user_pages() on anonymous pages.
> 
> This function is already implemented in PARISC and ARM.  Also see
> Documentation/cachetlb.txt.

**GRRRRR**

From <linux/highmem.h>:

#ifndef ARCH_HAS_FLUSH_ANON_PAGE
static inline void flush_anon_page(struct vm_area_struct *vma, struct page *page
, unsigned long vmaddr)
{
}
#endif

See why such stuff is happening?  Providing a default implemntation may be
fine for i.e. strcpy() where it's only about providing a better mouse
trap than the standard one in lib/string.c.  It's an awfully bad idea for
something that matters for correctness such as flush_anon_page() ...

Oh well, I'm looking into it.  Fortunately I've written reasonable
infrastructure during the previous round of the cache alias wars, so this
case should be reasonably simple to solve.  First cut of patch below.

The other thing I still need to understand is why nobody actually seems
to have triggered this bug on MIPS so far.  I suppose our implementation
of flush_dcache_page() may have done a successful job at papering it
which means there might be some performance getting lost there as well.

Was this one found by code inspection or actually triggered by like FUSE?

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index 31819c5..f565608 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -3,7 +3,8 @@
  * License.  See the file "COPYING" in the main directory of this archive
  * for more details.
  *
- * Copyright (C) 1994 - 2003 by Ralf Baechle
+ * Copyright (C) 1994 - 2003, 07 by Ralf Baechle (ralf@linux-mips.org)
+ * Copyright (C) 2007 MIPS Technologies, Inc.
  */
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -88,6 +89,23 @@ void __flush_dcache_page(struct page *page)
 
 EXPORT_SYMBOL(__flush_dcache_page);
 
+void __flush_anon_page(struct vm_area_struct *vma, struct page *page,
+	unsigned long vmaddr)
+{
+	if (!cpu_has_dc_aliases)
+		return;
+
+	if (pages_do_alias((unsigned long)page_address(page), vmaddr)) {
+		void *kaddr;
+
+		kaddr = kmap_coherent(page, vmaddr);
+		flush_data_cache_page((unsigned long)kaddr);
+		kunmap_coherent(kaddr);
+	}
+}
+
+EXPORT_SYMBOL(__flush_anon_page);
+
 void __update_cache(struct vm_area_struct *vma, unsigned long address,
 	pte_t pte)
 {
diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index 25abe91..e9951c0 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -123,7 +123,7 @@ static void __init kmap_coherent_init(void)
 static inline void kmap_coherent_init(void) {}
 #endif
 
-static inline void *kmap_coherent(struct page *page, unsigned long addr)
+void *kmap_coherent(struct page *page, unsigned long addr)
 {
 	enum fixed_addresses idx;
 	unsigned long vaddr, flags, entrylo;
@@ -177,7 +177,7 @@ static inline void *kmap_coherent(struct page *page, unsigned long addr)
 
 #define UNIQUE_ENTRYHI(idx) (CKSEG0 + ((idx) << (PAGE_SHIFT + 1)))
 
-static inline void kunmap_coherent(struct page *page)
+void kunmap_coherent(struct page *page)
 {
 #ifndef CONFIG_MIPS_MT_SMTC
 	unsigned int wired;
diff --git a/include/asm-mips/cacheflush.h b/include/asm-mips/cacheflush.h
index 0ddada3..1bd3310 100644
--- a/include/asm-mips/cacheflush.h
+++ b/include/asm-mips/cacheflush.h
@@ -48,6 +48,16 @@ static inline void flush_dcache_page(struct page *page)
 #define flush_dcache_mmap_lock(mapping)		do { } while (0)
 #define flush_dcache_mmap_unlock(mapping)	do { } while (0)
 
+#define ARCH_HAS_FLUSH_ANON_PAGE
+static inline void flush_anon_page(struct vm_area_struct *vma,
+	struct page *page, unsigned long vmaddr)
+{
+	extern void __flush_anon_page(struct vm_area_struct *vma,
+	                              struct page *, unsigned long);
+	if (PageAnon(page))
+		__flush_anon_page(vma, page, vmaddr);
+}
+
 static inline void flush_icache_page(struct vm_area_struct *vma,
 	struct page *page)
 {
@@ -86,4 +96,7 @@ extern void (*flush_data_cache_page)(unsigned long addr);
 /* Run kernel code uncached, useful for cache probing functions. */
 unsigned long __init run_uncached(void *func);
 
+extern void *kmap_coherent(struct page *page, unsigned long addr);
+extern void kunmap_coherent(struct page *page);
+
 #endif /* _ASM_CACHEFLUSH_H */

WARNING: multiple messages have this Message-ID (diff)
From: Ralf Baechle <ralf@linux-mips.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-mips@linux-mips.org, Ravi.Pratap@hillcrestlabs.com
Subject: Re: flush_anon_page for MIPS
Date: Fri, 23 Mar 2007 14:19:39 +0000	[thread overview]
Message-ID: <20070323141939.GB17311@linux-mips.org> (raw)
Message-ID: <20070323141939.OlgAfAJL6xDh9NFg87vEYkUcN-eHM4L05nHj5_NI33s@z> (raw)
In-Reply-To: <E1HUVlw-00034H-00@dorka.pomaz.szeredi.hu>

On Thu, Mar 22, 2007 at 11:28:40PM +0100, Miklos Szeredi wrote:

> It seems that MIPS needs to implement flush_anon_page() for correct
> operation of get_user_pages() on anonymous pages.
> 
> This function is already implemented in PARISC and ARM.  Also see
> Documentation/cachetlb.txt.

**GRRRRR**

  reply	other threads:[~2007-03-23 14:19 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-22 22:28 flush_anon_page for MIPS Miklos Szeredi
2007-03-23 14:19 ` Ralf Baechle [this message]
2007-03-23 14:19   ` Ralf Baechle
2007-03-23 14:47   ` Franck Bui-Huu
2007-03-23 15:20     ` Ralf Baechle
2007-03-23 21:00       ` Franck Bui-Huu
2007-03-23 21:20         ` Ralf Baechle
2007-03-23 14:51   ` Ravi Pratap
2007-03-23 14:51     ` Ravi Pratap
2007-03-23 15:41     ` Atsushi Nemoto
2007-03-23 15:47       ` Ravi Pratap
2007-03-23 15:47         ` Ravi Pratap
2007-03-23 15:50         ` Atsushi Nemoto
2007-03-26 13:31           ` Atsushi Nemoto
2007-03-26 13:36             ` Ravi Pratap
2007-03-26 13:36               ` Ravi Pratap
2007-03-26 14:33               ` Ralf Baechle
2007-03-23 15:56         ` Ralf Baechle
2007-03-23 15:59           ` Ravi Pratap
2007-03-23 15:59             ` Ravi Pratap
2007-03-23 16:11             ` Maxime Bizon
2007-03-23 16:12             ` Franck Bui-Huu
2007-03-23 16:17               ` Ravi Pratap
2007-03-23 16:17                 ` Ravi Pratap
2007-03-23 19:06         ` Ralf Baechle
2007-03-23 22:17           ` Ravi Pratap
2007-03-23 22:17             ` Ravi Pratap
2007-03-23 23:36             ` Ralf Baechle
2007-03-24  0:03               ` David Daney
2007-03-26 13:33                 ` Ravi Pratap
2007-03-26 13:33                   ` Ravi Pratap
2007-03-26 14:05                   ` Ralf Baechle
2007-03-26 22:38                     ` Ravi Pratap
2007-03-26 22:38                       ` Ravi Pratap
2007-03-26 23:24                     ` Ravi Pratap
2007-03-26 23:24                       ` Ravi Pratap
2007-03-27  3:17                       ` Atsushi Nemoto
2007-03-27 14:51                         ` Ravi Pratap
2007-03-27 14:51                           ` Ravi Pratap
2007-03-28 14:25                         ` Ravi Pratap
2007-03-28 14:25                           ` Ravi Pratap
2007-03-28 17:10                           ` David Daney
2007-03-28 18:17                             ` Ravi Pratap
2007-03-28 18:17                               ` Ravi Pratap
2007-03-28 20:34                             ` Ralf Baechle
2007-03-28 20:38                               ` David Daney
2007-03-23 15:01   ` Franck Bui-Huu
2007-03-23 15:36     ` Ralf Baechle
2007-03-23 20:52       ` Franck Bui-Huu
2007-03-23 16:22   ` Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070323141939.GB17311@linux-mips.org \
    --to=ralf@linux-mips.org \
    --cc=Ravi.Pratap@hillcrestlabs.com \
    --cc=linux-mips@linux-mips.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox