Skip to contentSkip to conversation

parisc: fix cache flushing in kmap/kunmap API implementation for aio

PATCH In review Helge Deller 3 participants 3 messages Dec 1, 2013 – Dec 4, 2013 PA-RISC architecture

lore
Share & tools
lore

Helge Deller

author maintainer
#0 lore raw

PATCH 1/1 parisc: fix cache flushing in kmap/kunmap API implementation for aio

This is now the next try to fix the aio bug in our kmap/kunmap implementation. It's basically stuff from

@James: Does this looks better? Can we get a signed-off or Acked-by from you on this patch?
The old mail thread was named "aio: fix D-cache aliasing issues"..

Dave & James: You can see my current patch description below. I would be happy if one of you could come up with a better one...

Tested on my C8000 and works OK.

Helge

-------------------
From: John David Anglin <[email protected]>

The aio-stress testcase revealed the problems in our kmap/kunmap API, that we didn't flushed the dcache page during kmap() on PA8800/PA8900 CPUs, and always needed to flush the kernel dcache at kunmap(), even if the CPU doesn't require conherency (< PA8800).

arch/parisc/include/asm/cacheflush.h +10 -8
index f0e2784..8ddab5e 100644
@@ -125,42 +125,44 @@ flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vma
 void mark_rodata_ro(void);
 #endif
 
-#ifdef CONFIG_PA8X00
-/* Only pa8800, pa8900 needs this */
-
 #include <asm/kmap_types.h>
 
 #define ARCH_HAS_KMAP
 
-void kunmap_parisc(void *addr);
-
 static inline void *kmap(struct page *page)
 {
 	might_sleep();
+	/* Flush dcache page on CPUs that require coherency. */
+	if (parisc_requires_coherency())
+		flush_dcache_page(page);
 	return page_address(page);
 }
 
 static inline void kunmap(struct page *page)
 {
-	kunmap_parisc(page_address(page));
+	/* Always flush kernel dcache, even if CPU doesn't require conherency.
+	 * Needed on CPUs < PA8800/PA8900 for AIO support. */
+	flush_kernel_dcache_page_addr(page_address(page));
 }
 
 static inline void *kmap_atomic(struct page *page)
 {
 	pagefault_disable();
+	/* Flush dcache page on CPUs that require coherency. */
+	if (parisc_requires_coherency())
+		flush_dcache_page(page);
 	return page_address(page);
 }
 
 static inline void __kunmap_atomic(void *addr)
 {
-	kunmap_parisc(addr);
+	flush_kernel_dcache_page_addr(addr);
 	pagefault_enable();
 }
 
 #define kmap_atomic_prot(page, prot)	kmap_atomic(page)
 #define kmap_atomic_pfn(pfn)	kmap_atomic(pfn_to_page(pfn))
 #define kmap_atomic_to_page(ptr)	virt_to_page(ptr)
-#endif
 
 #endif /* _PARISC_CACHEFLUSH_H */
 
arch/parisc/include/asm/processor.h +2 -2
index cc2290a..9cbee39 100644
@@ -106,6 +106,7 @@ struct cpuinfo_parisc {
 };
 
 extern struct system_cpuinfo_parisc boot_cpu_data;
+extern int parisc_coherent_cpu;
 DECLARE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 #define CPU_HVERSION ((boot_cpu_data.hversion >> 4) & 0x0FFF)
@@ -338,8 +339,7 @@ extern unsigned long get_wchan(struct task_struct *p);
 static inline int parisc_requires_coherency(void)
 {
 #ifdef CONFIG_PA8X00
-	return (boot_cpu_data.cpu_type == mako) ||
-		(boot_cpu_data.cpu_type == mako2);
+	return parisc_coherent_cpu;
 #else
 	return 0;
 #endif
arch/parisc/kernel/cache.c +8 -20
index c035673..3c6899f 100644
@@ -391,8 +391,6 @@ EXPORT_SYMBOL(flush_kernel_dcache_page_addr);
 void clear_user_page(void *vto, unsigned long vaddr, struct page *page)
 {
 	clear_page_asm(vto);
-	if (!parisc_requires_coherency())
-		flush_kernel_dcache_page_asm(vto);
 }
 EXPORT_SYMBOL(clear_user_page);
 
@@ -401,28 +399,18 @@ void copy_user_page(void *vto, void *vfrom, unsigned long vaddr,
 {
 	/* Copy using kernel mapping.  No coherency is needed
 	   (all in kmap/kunmap) on machines that don't support
-	   non-equivalent aliasing.  However, the `from' page
-	   needs to be flushed before it can be accessed through
-	   the kernel mapping. */
-	preempt_disable();
-	flush_dcache_page_asm(__pa(vfrom), vaddr);
-	preempt_enable();
+	   non-equivalent aliasing.  On other machines, the
+	   `from' page needs to be flushed before it can be
+	   accessed through the kernel mapping. */
+	if (!parisc_requires_coherency()) {
+		preempt_disable();
+		flush_dcache_page_asm(__pa(vfrom), vaddr);
+		preempt_enable();
+	}
 	copy_page_asm(vto, vfrom);
-	if (!parisc_requires_coherency())
-		flush_kernel_dcache_page_asm(vto);
 }
 EXPORT_SYMBOL(copy_user_page);
 
-#ifdef CONFIG_PA8X00
-
-void kunmap_parisc(void *addr)
-{
-	if (parisc_requires_coherency())
-		flush_kernel_dcache_page_addr(addr);
-}
-EXPORT_SYMBOL(kunmap_parisc);
-#endif
-
 void purge_tlb_entries(struct mm_struct *mm, unsigned long addr)
 {
 	unsigned long flags;
arch/parisc/kernel/processor.c +10 -0
index b68d977..f1b8c48 100644
@@ -45,6 +45,11 @@
 struct system_cpuinfo_parisc boot_cpu_data __read_mostly;
 EXPORT_SYMBOL(boot_cpu_data);
 
+#ifdef CONFIG_PA8X00
+int parisc_coherent_cpu __read_mostly;
+EXPORT_SYMBOL(parisc_coherent_cpu);
+#endif
+
 DEFINE_PER_CPU(struct cpuinfo_parisc, cpu_data);
 
 extern int update_cr16_clocksource(void);	/* from time.c */
@@ -271,6 +276,11 @@ void __init collect_boot_cpu_data(void)
 		printk(KERN_INFO "model %s\n",
 			boot_cpu_data.pdc.sys_model_name);
 
+#ifdef CONFIG_PA8X00
+	parisc_coherent_cpu = (boot_cpu_data.cpu_type == mako) ||
+			      (boot_cpu_data.cpu_type == mako2);
+#endif
+
 	boot_cpu_data.hversion =  boot_cpu_data.pdc.model.hversion;
 	boot_cpu_data.sversion =  boot_cpu_data.pdc.model.sversion;
 
maintainer
#1 lore raw
Quoted 2 lines · Helge Deller

This is now the next try to fix the aio bug in our kmap/kunmap implementation. It's basically stuff from

I thought the implementation required us to move to coherency management in the kmap API?, in which case ...

[...]

arch/parisc/include/asm/cacheflush.h flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vma · line 139

Quoted 10 lines
arch/parisc/include/asm/cacheflush.h +3 -2
-void kunmap_parisc(void *addr);
-
 static inline void *kmap(struct page *page)
 {
 	might_sleep();
+	/* Flush dcache page on CPUs that require coherency. */
+	if (parisc_requires_coherency())
+		flush_dcache_page(page);
 	return page_address(page);
 }

We have to call flush_dcache_page() unconditionally here. It's the API that coheres all the user space mappings. It has to be called somehow before the kernel begins operating on the mapped address.

Quoted 6 lines

static inline void kunmap(struct page *page)
{

arch/parisc/include/asm/cacheflush.h +3 -1
-	kunmap_parisc(page_address(page));
+	/* Always flush kernel dcache, even if CPU doesn't require conherency.
+	 * Needed on CPUs < PA8800/PA8900 for AIO support. */
+	flush_kernel_dcache_page_addr(page_address(page));

arch/parisc/include/asm/cacheflush.h flush_anon_page(struct vm_area_struct *vma, struct page *page, unsigned long vma · line 155

Quoted 10 lines
arch/parisc/include/asm/cacheflush.h +3 -0
 }
 
 static inline void *kmap_atomic(struct page *page)
 {
 	pagefault_disable();
+	/* Flush dcache page on CPUs that require coherency. */
+	if (parisc_requires_coherency())
+		flush_dcache_page(page);
 	return page_address(page);
 }

And unconditionally here too.

And if we do that, I don't see that we need special code for the pa88/8900 any more.

James

#2 lore raw
Quoted 6 lines · James Bottomley
Quoted 2 lines · Helge Deller

This is now the next try to fix the aio bug in our kmap/kunmap implementation. It's basically stuff from

I thought the implementation required us to move to coherency management in the kmap API?, in which case ...

This is what I've been saying to Helge.

We don't fully understand why we need to flush unconditionally to fix the aio bug in kunmap. I can see it for coherency and when different non coherent caches are involved (e.g., I & D). Could this be SMP related?

I think if we understand the kunmap case, we will also understand the kmap case. At the moment, the fix is just based on testing.

Dave
--
John David Anglin [email protected]