Cache maintenance on VIPT caches
Subsystems (2)
cover Cache maintenance on VIPT caches
Hi,
The first and third patches have already been posted in the same form. The second patch have been modified to handle all the VIPT caches via __sync_icache_dcache(). The initial use case for this patch was dealing with an SMP race condition but following suggestions from Rabin, it was extended to cover ARMv6 onwards, both UP and SMP.
Any Tested-by's are welcome.
Thanks.
Catalin Marinas (3):
ARM: Assume new page cache pages have dirty D-cache
ARM: Introduce __sync_icache_dcache() for VIPT caches
ARM: Use lazy cache flushing on ARMv7 SMP systems
arch/arm/include/asm/cacheflush.h | 6 ++--- arch/arm/include/asm/pgtable.h | 26 +++++++++++++++++++-- arch/arm/include/asm/smp_plat.h | 4 +++ arch/arm/include/asm/tlbflush.h | 12 ++++++++-- arch/arm/mm/copypage-v4mc.c | 2 +- arch/arm/mm/copypage-v6.c | 2 +- arch/arm/mm/copypage-xscale.c | 2 +- arch/arm/mm/dma-mapping.c | 6 +++++ arch/arm/mm/fault-armv.c | 8 +++--- arch/arm/mm/flush.c | 46 +++++++++++++++++++++++++++++-------- 10 files changed, 89 insertions(+), 25 deletions(-)
Signature · 1 line
Catalin
PATCH 1/3 ARM: Assume new page cache pages have dirty D-cache
There are places in Linux where writes to newly allocated page cache pages happen without a subsequent call to flush_dcache_page() (several PIO drivers including USB HCD). This patch changes the meaning of PG_arch_1 to be PG_dcache_clean and always flush the D-cache for a newly mapped page in update_mmu_cache().
The patch also sets the PG_arch_1 bit in the DMA cache maintenance function to avoid additional cache flushing in update_mmu_cache().
--- arch/arm/include/asm/cacheflush.h | 6 +++--- arch/arm/include/asm/tlbflush.h | 2 +- arch/arm/mm/copypage-v4mc.c | 2 +- arch/arm/mm/copypage-v6.c | 2 +- arch/arm/mm/copypage-xscale.c | 2 +- arch/arm/mm/dma-mapping.c | 6 ++++++ arch/arm/mm/fault-armv.c | 4 ++-- arch/arm/mm/flush.c | 3 ++- 8 files changed, 17 insertions(+), 10 deletions(-)
arch/arm/include/asm/cacheflush.h +3 -3
@@ -137,10 +137,10 @@#endif /* -* This flag is used to indicate that the page pointed to by a pte -* is dirty and requires cleaning before returning it to the user. +* This flag is used to indicate that the page pointed to by a pte is clean +* and does not require cleaning before returning it to the user. */ -#define PG_dcache_dirty PG_arch_1 +#define PG_dcache_clean PG_arch_1/* * MM Cache Management
arch/arm/include/asm/tlbflush.h +1 -1
@@ -552,7 +552,7 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);#endif /* -* if PG_dcache_dirty is set for the page, we need to ensure that any +* If PG_dcache_clean is not set for the page, we need to ensure that any * cache entries for the kernels virtual memory range are written * back to the page. */
arch/arm/mm/copypage-v4mc.c +1 -1
@@ -73,7 +73,7 @@ void v4_mc_copy_user_highpage(struct page *to, struct page *from, {void *kto =kmap_atomic (to,KM_USER1 ); -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);spin_lock (&minicache_lock);
arch/arm/mm/copypage-v6.c +1 -1
@@ -79,7 +79,7 @@ static void v6_copy_user_highpage_aliasing(struct page *to,unsigned int offset =CACHE_COLOUR (vaddr);unsigned long kfrom, kto; -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);/* FIXME: not highmem safe */
arch/arm/mm/copypage-xscale.c +1 -1
@@ -95,7 +95,7 @@ void xscale_mc_copy_user_highpage(struct page *to, struct page *from, {void *kto =kmap_atomic (to,KM_USER1 ); -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);spin_lock (&minicache_lock);
arch/arm/mm/dma-mapping.c +6 -0
@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,outer_inv_range (paddr, paddr + size);dma_cache_maint_page (page, off, size, dir, dmac_unmap_area); + +/* +* Mark the D-cache clean for this page to avoid extra flushing. +*/ +if (dir !=DMA_TO_DEVICE ) +set_bit (PG_dcache_clean, &page->flags); }EXPORT_SYMBOL (___dma_page_dev_to_cpu);
arch/arm/mm/fault-armv.c +2 -2
@@ -141,7 +141,7 @@ make_coherent(struct address_space *mapping, struct vm_area_struct *vma,* a page table, or changing an existing PTE. Basically, there are two * things that we need to take care of: * -* 1. If PG_dcache_dirty is set for the page, we need to ensure +* 1. If PG_dcache_clean is not set for the page, we need to ensure * that any cache entries for the kernels virtual memory * range are written back to the page. * 2. If we have multiple shared mappings of the same space in
@@ -169,7 +169,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr, mapping =page_mapping (page);#ifndef CONFIG_SMP -if (test_and_clear_bit (PG_dcache_dirty, &page->flags)) +if (!test_and_set_bit (PG_dcache_clean, &page->flags))__flush_dcache_page (mapping, page);#endif if (mapping) {
arch/arm/mm/flush.c +2 -1
@@ -248,7 +248,7 @@ void flush_dcache_page(struct page *page)#ifndef CONFIG_SMP if (!PageHighMem (page) && mapping && !mapping_mapped (mapping)) -set_bit (PG_dcache_dirty, &page->flags); +clear_bit (PG_dcache_clean, &page->flags); else#endif {
@@ -257,6 +257,7 @@ void flush_dcache_page(struct page *page)__flush_dcache_aliases (mapping, page); else if (mapping) __flush_icache_all(); +set_bit (PG_dcache_clean, &page->flags); } }EXPORT_SYMBOL (flush_dcache_page);
PATCH 1/3 ARM: Assume new page cache pages have dirty D-cache
arch/arm/mm/dma-mapping.c void ___dma_page_dev_to_cpu(struct page *page, unsigned long off, · line 516
Quoted 10 lines · Catalin Marinas
arch/arm/mm/dma-mapping.c +6
-0@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,outer_inv_range (paddr, paddr + size);dma_cache_maint_page (page, off, size, dir, dmac_unmap_area); + +/* +* Mark the D-cache clean for this page to avoid extra flushing. +*/ +if (dir !=DMA_TO_DEVICE ) +set_bit (PG_dcache_clean, &page->flags);
off + size does not necessarily cover the full page. We probably shouldn't be setting it as clean unless it does?
Also, off + size can cover multiple physically contiguous pages, so perhaps they too can be set as clean if they're fully covered.
Rabin
PATCH 1/3 ARM: Assume new page cache pages have dirty D-cache
Quoted 4 lines · Rabin Vincent
Quoted 3 lines · Catalin Marinas
(quoted diff) +0
-0@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off, outer_inv_range(paddr, paddr + size);
Quoted 10 lines
Review void ___dma_page_dev_to_cpu(struct page *page, unsigned long off, · line 516
Quoted 7 lines
(quoted diff) +6
-0dma_cache_maint_page(page, off, size, dir, dmac_unmap_area); + + /* + * Mark the D-cache clean for this page to avoid extra flushing. + */ + if (dir != DMA_TO_DEVICE) + set_bit(PG_dcache_clean, &page->flags);off + size does not necessarily cover the full page. We probably shouldn't be setting it as clean unless it does?
Good point. See below for an updated patch.
Quoted 2 lines
Also, off + size can cover multiple physically contiguous pages, so perhaps they too can be set as clean if they're fully covered.
While it may be possible, I don't think that's common. I would only care about the "page" argument, otherwise you have to do some conversion to find the page structure and it may not be worth.
ARM: Assume new page cache pages have dirty D-cache
From: Catalin Marinas <[email protected]>
There are places in Linux where writes to newly allocated page cache pages happen without a subsequent call to flush_dcache_page() (several PIO drivers including USB HCD). This patch changes the meaning of PG_arch_1 to be PG_dcache_clean and always flush the D-cache for a newly mapped page in update_mmu_cache().
The patch also sets the PG_arch_1 bit in the DMA cache maintenance function to avoid additional cache flushing in update_mmu_cache().
--- arch/arm/include/asm/cacheflush.h | 6 +++--- arch/arm/include/asm/tlbflush.h | 2 +- arch/arm/mm/copypage-v4mc.c | 2 +- arch/arm/mm/copypage-v6.c | 2 +- arch/arm/mm/copypage-xscale.c | 2 +- arch/arm/mm/dma-mapping.c | 6 ++++++ arch/arm/mm/fault-armv.c | 4 ++-- arch/arm/mm/flush.c | 3 ++- 8 files changed, 17 insertions(+), 10 deletions(-)
arch/arm/include/asm/cacheflush.h +3 -3
@@ -137,10 +137,10 @@#endif /* -* This flag is used to indicate that the page pointed to by a pte -* is dirty and requires cleaning before returning it to the user. +* This flag is used to indicate that the page pointed to by a pte is clean +* and does not require cleaning before returning it to the user. */ -#define PG_dcache_dirty PG_arch_1 +#define PG_dcache_clean PG_arch_1/* * MM Cache Management
arch/arm/include/asm/tlbflush.h +1 -1
@@ -552,7 +552,7 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);#endif /* -* if PG_dcache_dirty is set for the page, we need to ensure that any +* If PG_dcache_clean is not set for the page, we need to ensure that any * cache entries for the kernels virtual memory range are written * back to the page. */
arch/arm/mm/copypage-v4mc.c +1 -1
@@ -73,7 +73,7 @@ void v4_mc_copy_user_highpage(struct page *to, struct page *from, {void *kto =kmap_atomic (to,KM_USER1 ); -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);spin_lock (&minicache_lock);
arch/arm/mm/copypage-v6.c +1 -1
@@ -79,7 +79,7 @@ static void v6_copy_user_highpage_aliasing(struct page *to,unsigned int offset =CACHE_COLOUR (vaddr);unsigned long kfrom, kto; -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);/* FIXME: not highmem safe */
arch/arm/mm/copypage-xscale.c +1 -1
@@ -95,7 +95,7 @@ void xscale_mc_copy_user_highpage(struct page *to, struct page *from, {void *kto =kmap_atomic (to,KM_USER1 ); -if (test_and_clear_bit (PG_dcache_dirty, &from->flags)) +if (!test_and_set_bit (PG_dcache_clean, &from->flags))__flush_dcache_page (page_mapping (from), from);spin_lock (&minicache_lock);
arch/arm/mm/dma-mapping.c +6 -0
@@ -508,6 +508,12 @@ void ___dma_page_dev_to_cpu(struct page *page, unsigned long off,outer_inv_range (paddr, paddr + size);dma_cache_maint_page (page, off, size, dir, dmac_unmap_area); + +/* +* Mark the D-cache clean for this page to avoid extra flushing. +*/ +if (dir !=DMA_TO_DEVICE && off ==0 && size >=PAGE_SIZE ) +set_bit (PG_dcache_clean, &page->flags); }EXPORT_SYMBOL (___dma_page_dev_to_cpu);
arch/arm/mm/fault-armv.c +2 -2
@@ -141,7 +141,7 @@ make_coherent(struct address_space *mapping, struct vm_area_struct *vma,* a page table, or changing an existing PTE. Basically, there are two * things that we need to take care of: * -* 1. If PG_dcache_dirty is set for the page, we need to ensure +* 1. If PG_dcache_clean is not set for the page, we need to ensure * that any cache entries for the kernels virtual memory * range are written back to the page. * 2. If we have multiple shared mappings of the same space in
@@ -169,7 +169,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr, mapping =page_mapping (page);#ifndef CONFIG_SMP -if (test_and_clear_bit (PG_dcache_dirty, &page->flags)) +if (!test_and_set_bit (PG_dcache_clean, &page->flags))__flush_dcache_page (mapping, page);#endif if (mapping) {
arch/arm/mm/flush.c +2 -1
@@ -248,7 +248,7 @@ void flush_dcache_page(struct page *page)#ifndef CONFIG_SMP if (!PageHighMem (page) && mapping && !mapping_mapped (mapping)) -set_bit (PG_dcache_dirty, &page->flags); +clear_bit (PG_dcache_clean, &page->flags); else#endif {
@@ -257,6 +257,7 @@ void flush_dcache_page(struct page *page)__flush_dcache_aliases (mapping, page); else if (mapping) __flush_icache_all(); +set_bit (PG_dcache_clean, &page->flags); } }EXPORT_SYMBOL (flush_dcache_page);
PATCH 2/3 ARM: Introduce __sync_icache_dcache() for VIPT caches
In reply to Catalin MarinasOn SMP systems, there is a small chance of a PTE becoming visible to a different CPU before the current cache maintenance operations in update_mmu_cache(). To avoid this, cache maintenance must be handled in set_pte_at() (similar to IA-64 and PowerPC).
This patch provides a unified VIPT cache handling mechanism and implements the __sync_icache_dcache() function for ARMv6 onwards architectures. It is called from set_pte_at() and replaces the update_mmu_cache(). The latter is still used on VIVT hardware where a vm_area_struct is required.
--- arch/arm/include/asm/pgtable.h | 26 +++++++++++++++++++++++--- arch/arm/include/asm/tlbflush.h | 10 +++++++++- arch/arm/mm/fault-armv.c | 4 ++-- arch/arm/mm/flush.c | 30 ++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 6 deletions(-)
arch/arm/include/asm/pgtable.h +23 -3
@@ -278,9 +278,24 @@ extern struct page *empty_zero_page;#define set_pte_ext (ptep,pte,ext) cpu_set_pte_ext(ptep,pte,ext) -#define set_pte_at (mm,addr,ptep,pteval) do { \ - set_pte_ext(ptep, pteval, (addr) >= TASK_SIZE ? 0 : PTE_EXT_NG); \ - } while (0) +#if __LINUX_ARM_ARCH__ <6 +static inline void __sync_icache_dcache (pte_t pteval) +{ +} +#else +extern void __sync_icache_dcache (pte_t pteval); +#endif + +static inline void set_pte_at (struct mm_struct *mm,unsigned long addr, +pte_t *ptep,pte_t pteval) +{ +if (addr >=TASK_SIZE ) +set_pte_ext (ptep, pteval,0 ); +else { +__sync_icache_dcache (pteval); +set_pte_ext (ptep, pteval,PTE_EXT_NG ); + } +}/* * The following only work if pte_present() is true.
@@ -290,8 +305,13 @@ extern struct page *empty_zero_page;#define pte_write (pte) (pte_val(pte) & L_PTE_WRITE)#define pte_dirty (pte) (pte_val(pte) & L_PTE_DIRTY)#define pte_young (pte) (pte_val(pte) & L_PTE_YOUNG) +#define pte_exec (pte) (pte_val(pte) & L_PTE_EXEC)#define pte_special (pte) (0) +#define pte_present_user (pte) \ + ((pte_val(pte) & (L_PTE_PRESENT | L_PTE_USER)) == \ + (L_PTE_PRESENT | L_PTE_USER)) +#define PTE_BIT_FUNC (fn,op) \ static inline pte_t pte_##fn(pte_t pte) { pte_val(pte) op; return pte; }
arch/arm/include/asm/tlbflush.h +9 -1
@@ -554,10 +554,18 @@ extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);/* * If PG_dcache_clean is not set for the page, we need to ensure that any * cache entries for the kernels virtual memory range are written -* back to the page. +* back to the page. On ARMv6 and later, the cache coherency is handled via +* the set_pte_at() function. */ +#if __LINUX_ARM_ARCH__ <6 extern void update_mmu_cache (struct vm_area_struct *vma,unsigned long addr,pte_t *ptep); +#else +static inline void update_mmu_cache (struct vm_area_struct *vma, +unsigned long addr,pte_t *ptep) +{ +} +#endif #endif
arch/arm/mm/fault-armv.c +2 -2
@@ -28,6 +28,7 @@static unsigned long shared_pte_mask =L_PTE_MT_BUFFERABLE ; +#if __LINUX_ARM_ARCH__ <6 /* * We take the easy way out of this problem - we make the * PTE uncacheable. However, we leave the write buffer on.
@@ -168,10 +169,8 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr,return ; mapping =page_mapping (page); -#ifndef CONFIG_SMP if (!test_and_set_bit (PG_dcache_clean, &page->flags))__flush_dcache_page (mapping, page); -#endif if (mapping) {if (cache_is_vivt ())make_coherent (mapping, vma, addr, ptep, pfn);
@@ -179,6 +178,7 @@ void update_mmu_cache(struct vm_area_struct *vma, unsigned long addr, __flush_icache_all(); } } +#endif /* __LINUX_ARM_ARCH__ < 6 */ /* * Check whether the write buffer has physical address aliasing
arch/arm/mm/flush.c +30 -0
@@ -215,6 +215,36 @@ static void __flush_dcache_aliases(struct address_space *mapping, struct page *pflush_dcache_mmap_unlock (mapping); } +#if __LINUX_ARM_ARCH__ >=6 +void __sync_icache_dcache (pte_t pteval) +{ +unsigned long pfn; +struct page *page; +struct address_space *mapping; + +if (!pte_present_user (pteval)) +return ; +if (cache_is_vipt_nonaliasing () && !pte_exec (pteval)) +/* only flush non-aliasing VIPT caches for exec mappings */ +return ; + pfn =pte_pfn (pteval); +if (!pfn_valid (pfn)) +return ; + + page =pfn_to_page (pfn); +if (cache_is_vipt_aliasing ()) + mapping =page_mapping (page); +else + mapping =NULL ; + +if (!test_and_set_bit (PG_dcache_clean, &page->flags)) +__flush_dcache_page (mapping, page); +/* pte_exec() already checked above for non-aliasing VIPT cache */ +if (cache_is_vipt_nonaliasing () ||pte_exec (pteval)) +__flush_icache_all (); +} +#endif +/* * Ensure cache coherency between kernel mapping and userspace mapping * of this page.
PATCH 3/3 ARM: Use lazy cache flushing on ARMv7 SMP systems
In reply to Catalin MarinasARMv7 processors like Cortex-A9 broadcast the cache maintenance operations in hardware. This patch allows the flush_dcache_page/update_mmu_cache pair to work in lazy flushing mode similar to the UP case.
Note that cache flushing on SMP systems now takes place via the set_pte_at() call (__sync_icache_dcache) and there is no race with other CPUs executing code from the new PTE before the cache flushing took place.
--- arch/arm/include/asm/smp_plat.h | 4 ++++ arch/arm/mm/flush.c | 13 ++++--------- 2 files changed, 8 insertions(+), 9 deletions(-)
arch/arm/include/asm/smp_plat.h +4 -0
@@ -13,9 +13,13 @@ static inline int tlb_ops_need_broadcast(void) return ((read_cpuid_ext(CPUID_EXT_MMFR3) >> 12) & 0xf) < 2; } +#if !defined(CONFIG_SMP ) || __LINUX_ARM_ARCH__ >=7 +#define cache_ops_need_broadcast () 0 +#else static inline int cache_ops_need_broadcast (void ) {return ((read_cpuid_ext (CPUID_EXT_MMFR3 ) >>12 ) &0xf ) <1 ; } +#endif #endif
arch/arm/mm/flush.c +4 -9
@@ -17,6 +17,7 @@#include <asm/smp_plat.h> #include <asm/system.h> #include <asm/tlbflush.h> +#include <asm/smp_plat.h> #include "mm.h"
@@ -93,12 +94,10 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig#define flush_pfn_alias (pfn,vaddr) do { } while (0)#endif -#ifdef CONFIG_SMP static void flush_ptrace_access_other (void *args) {__flush_icache_all (); } -#endif static void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,
@@ -122,11 +121,9 @@ void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,if (vma->vm_flags &VM_EXEC ) {unsigned long addr = (unsigned long )kaddr;__cpuc_coherent_kern_range (addr, addr + len); -#ifdef CONFIG_SMP if (cache_ops_need_broadcast ())smp_call_function (flush_ptrace_access_other,NULL ,1 ); -#endif } }
@@ -276,12 +273,10 @@ void flush_dcache_page(struct page *page) mapping =page_mapping (page); -#ifndef CONFIG_SMP -if (!PageHighMem (page) && mapping && !mapping_mapped (mapping)) +if (!cache_ops_need_broadcast () && + !PageHighMem (page) && mapping && !mapping_mapped (mapping))clear_bit (PG_dcache_clean, &page->flags); - else -#endif - { +else {__flush_dcache_page (mapping, page);if (mapping &&cache_is_vivt ())__flush_dcache_aliases (mapping, page);
cover Cache maintenance on VIPT caches
In reply to Catalin MarinasQuoted 7 lines · Catalin Marinas
The first and third patches have already been posted in the same form. The second patch have been modified to handle all the VIPT caches via
__sync_icache_dcache(). The initial use case for this patch was dealing with an SMP race condition but following suggestions from Rabin, it was extended to cover ARMv6 onwards, both UP and SMP.Any Tested-by's are welcome.
This version also fixes the MMC rootfs init crashes, without the need for the flush_kernel_dcache_page() change:
Will you be submitting this patch for linux-next? I ask because the mmci patches that I posted convert that driver to use the sg_miter API (which uses flush_kernel_dcache_page() internally), and not do any flushing inside the driver itself. Or do you think it would be appropriate to have the driver call flush_dcache_page() explicitly? (Although this would be double flushing on systems with aliasing caches where flush_kernel_dcache_page() is not a no-op.)
Rabin
cover Cache maintenance on VIPT caches
Quoted 7 lines · Rabin VINCENT
Quoted 6 lines · Catalin Marinas
The first and third patches have already been posted in the same form. The second patch have been modified to handle all the VIPT caches via
__sync_icache_dcache(). The initial use case for this patch was dealing with an SMP race condition but following suggestions from Rabin, it was extended to cover ARMv6 onwards, both UP and SMP.
Quoted 6 lines
Quoted 1 line
Any Tested-by's are welcome.
This version also fixes the MMC rootfs init crashes, without the need for the
flush_kernel_dcache_page()change:Tested-by Rabin Vincent <[email protected]>
Thanks for testing.
Quoted 1 line
Will you be submitting this patch for linux-next?
I'd like to but I'm still waiting for Russell's feedback (the linux-next rules are that we shouldn't dump patches there that haven't been reviewed yet).
Quoted 7 lines
I ask because the
mmci patches that I posted convert that driver to use thesg_miterAPI (which usesflush_kernel_dcache_page()internally), and not do any flushing inside the driver itself. Or do you think it would be appropriate to have the driver callflush_dcache_page()explicitly? (Although this would be double flushing on systems with aliasing caches whereflush_kernel_dcache_page()is not a no-op.)
I'm not sure why sg_miter doesn't call flush_dcache_page() but flush_kernel_dcache_page() (I cc'ed Fujita as he seems to have added this code).
Anyway, we could actually set the PG_dcache_clean bit in flush_kernel_dcache_page() and thus avoid additional flushing when the page gets mapped to user space (on VIPT non-aliasing caches).
I think it's fine for your driver not to call flush_dcache_page() as long as a form of page flushing is done in the sg_miter API. The only thing we miss is ARM11MPCore with VIPT non-aliasing caches where flush_kernel_dcache_page() is a no-op, though we have to do non-lazy flushing because the cache maintenance operations aren't broadcast to the other CPUs.
There are several places where we can fix this - (1) implement flush_kernel_dcache_page() for ARM11MPCore, (2) use flush_dcache_page() in the sg_miter code or (3) call flush_dcache_page() in the mmci driver.
Signature · 1 line
Catalin
Cc'ed to linux-arch
Quoted 8 lines · On Fri, 16 Jul 2010 15:39:17 +0100 Catalin Marinas <[email protected]>…
Quoted 7 lines · Rabin VINCENT
Quoted 6 lines · Catalin Marinas
The first and third patches have already been posted in the same form. The second patch have been modified to handle all the VIPT caches via
__sync_icache_dcache(). The initial use case for this patch was dealing with an SMP race condition but following suggestions from Rabin, it was extended to cover ARMv6 onwards, both UP and SMP.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Any Tested-by's are welcome.
This version also fixes the MMC rootfs init crashes, without the need for the
flush_kernel_dcache_page()change:Tested-by Rabin Vincent <[email protected]>Thanks for testing.
This patchset for arm handles I/D coherency (and D aliasing) with PG_arch_1, right? If so, can you please send it to linux-arch? I guess that some non arm people might be interested in it.
Quoted 11 lines
Quoted 7 lines
I ask because the
mmci patches that I posted convert that driver to use thesg_miterAPI (which usesflush_kernel_dcache_page()internally), and not do any flushing inside the driver itself. Or do you think it would be appropriate to have the driver callflush_dcache_page()explicitly? (Although this would be double flushing on systems with aliasing caches whereflush_kernel_dcache_page()is not a no-op.)I'm not sure why
sg_miterdoesn't callflush_dcache_page()butflush_kernel_dcache_page()(I cc'ed Fujita as he seems to have added this code).
It is intended to handle D aliasing due to kmap. As cachetlb.txt says that it is assumed here that the user has no incoherent cached copies (it was already addressed elsewhere). So flush_dcache_page() does too much there.
Quoted 16 lines · FUJITA Tomonori
Quoted 13 lines · On Fri, 16 Jul 2010 15:39:17 +0100 Catalin Marinas <[email protected]>…
Quoted 12 lines · Rabin VINCENT
Quoted 7 lines · Catalin Marinas
The first and third patches have already been posted in the same form. The second patch have been modified to handle all the VIPT caches via
__sync_icache_dcache(). The initial use case for this patch was dealing with an SMP race condition but following suggestions from Rabin, it was extended to cover ARMv6 onwards, both UP and SMP.Any Tested-by's are welcome.
This version also fixes the MMC rootfs init crashes, without the need for the
flush_kernel_dcache_page()change:
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Tested-by Rabin Vincent <[email protected]>Thanks for testing.
This patchset for arm handles I/D coherency (and D aliasing) with
PG_arch_1, right? If so, can you please send it to linux-arch? I guess that some non arm people might be interested in it.
Yes, I will.
ARM uses lazy D-cache flushing as per cachetlb.txt. However, this file recommends that the meaning of PG_arch_1 is 'dirty' and the bit is set by flush_dcache_page(). It works fine if all PIO drivers would call flush_dcache_page() but there are many (the majority?) that don't and we end up with data corruption in user space. So rather than changing all the drivers or proposing a PIO API (tried this as well :)), we changed the meaning of PG_arch_1 to 'clean'. That's already done by PowerPC and IA-64.
This was discussed on linux-arch on several occasions and I also posted a patch to change the recommendations in cachetlb.txt
(http://thread.gmane.org/gmane.linux.kernel.cross-arch/6187).
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
I ask because the
mmci patches that I posted convert that driver to use thesg_miterAPI (which usesflush_kernel_dcache_page()internally), and not do any flushing inside the driver itself. Or do you think it would be appropriate to have the driver callflush_dcache_page()explicitly? (Although this would be double flushing on systems with aliasing caches whereflush_kernel_dcache_page()is not a no-op.)
Quoted 8 lines
Quoted 3 lines
I'm not sure why
sg_miterdoesn't callflush_dcache_page()butflush_kernel_dcache_page()(I cc'ed Fujita as he seems to have added this code).It is intended to handle D aliasing due to kmap. As cachetlb.txt says that it is assumed here that the user has no incoherent cached copies (it was already addressed elsewhere). So
flush_dcache_page()does too much there.
OK. So in this case a PIO driver using sg_miter should still call flush_dcache_page() as usual (or not at all as we change the meaning PG_arch_1 for the architectures where this matters).
Thanks.
Signature · 1 line
Catalin
Quoted 17 lines · Catalin Marinas
Quoted 16 lines · FUJITA Tomonori
Quoted 12 lines · On Fri, 16 Jul 2010 15:39:17 +0100 Catalin Marinas <[email protected]>…
Quoted 7 lines · Rabin VINCENT
I ask because the
mmci patches that I posted convert that driver to use thesg_miterAPI (which usesflush_kernel_dcache_page()internally), and not do any flushing inside the driver itself. Or do you think it would be appropriate to have the driver callflush_dcache_page()explicitly? (Although this would be double flushing on systems with aliasing caches whereflush_kernel_dcache_page()is not a no-op.)I'm not sure why
sg_miterdoesn't callflush_dcache_page()butflush_kernel_dcache_page()(I cc'ed Fujita as he seems to have added this code).
Quoted 8 lines
Quoted 4 lines
It is intended to handle D aliasing due to kmap. As cachetlb.txt says that it is assumed here that the user has no incoherent cached copies (it was already addressed elsewhere). So
flush_dcache_page()does too much there.OK. So in this case a PIO driver using
sg_mitershould still callflush_dcache_page()as usual (or not at all as we change the meaningPG_arch_1for the architectures where this matters).
Actually the flush_dcache_page() call in drivers is still useful on SMP systems where the cache maintenance must happen on the CPU that dirtied the cache (ARM11MPCore), though I have a workaround for this as well (doing a 'read-for-ownership' before flushing the D-cache).
Signature · 1 line
Catalin
Quoted 10 lines · FUJITA Tomonori
Quoted 3 lines · On Fri, 16 Jul 2010 15:39:17 +0100 Catalin Marinas <[email protected]>…
I'm not sure why
sg_miterdoesn't callflush_dcache_page()butflush_kernel_dcache_page()(I cc'ed Fujita as he seems to have added this code).It is intended to handle D aliasing due to kmap. As cachetlb.txt says that it is assumed here that the user has no incoherent cached copies (it was already addressed elsewhere). So
flush_dcache_page()does too much there.
A related question here - does flush_kernel_dcache_page() need to do anything for non-highmem pages? For highmem pages, on ARM, we already handle the cache flushing in kunmap().
Signature · 1 line
Catalin
Quoted 7 lines · Catalin Marinas
Quoted 6 lines · FUJITA Tomonori
Quoted 3 lines · On Fri, 16 Jul 2010 15:39:17 +0100 Catalin Marinas <[email protected]>…
I'm not sure why
sg_miterdoesn't callflush_dcache_page()butflush_kernel_dcache_page()(I cc'ed Fujita as he seems to have added this code).
Quoted 8 lines
Quoted 4 lines
It is intended to handle D aliasing due to kmap. As cachetlb.txt says that it is assumed here that the user has no incoherent cached copies (it was already addressed elsewhere). So
flush_dcache_page()does too much there.A related question here - does
flush_kernel_dcache_page()need to do anything for non-highmem pages? For highmem pages, on ARM, we already handle the cache flushing inkunmap().
Well, with your proposed patch we wouldn't anymore. And semantically we shouldn't have to. Even the fact that kunmap_atomic() does flush the cache on VIVT system is an implementation issue not a semantic definition (incidentally on VIPT it is not flushed).
Nicolas