Cache handling updates

Errata 411920 indicates that any "invalidate entire instruction cache" operation can fail if the right conditions are present. This is not limited just to those operations in flush.c, but elsewhere. Place the workaround in the already existing __flush_icache_all() function instead.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/include/asm/cacheflush.h |    5 +++++
 arch/arm/mm/context.c             |    4 ++--
 arch/arm/mm/flush.c               |   31 ++++++-------------------------
 3 files changed, 13 insertions(+), 27 deletions(-)

arch/arm/include/asm/cacheflush.h +0 -0

index 3147f75..af0624c 100644

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mm/flush.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

arch/arm/mm/flush.c +3 -0

index dc66f86..9770e27 100644
@@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 		    : "cc");
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 
 void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)

Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache) caused by the dynamic linker changing protection on text pages to write GOT entries. The problem is due to an interaction between the write faulting code providing new anonymous pages (which haven't had their I-cache cleaned due to no VM_EXEC) and change_protection() not doing any kind of I-cache maintainence.

Work around this by flushing the I-cache in flush_cache_range() for VM_EXEC VMAs (which is what we have when the region is being made executable again.) This ensures that the I-cache will be up to date with the new COW'd pages.

Russell King (11):
ARM: Reduce __flush_dcache_page() visibility
ARM: Fix errata 411920 workarounds
ARM: move __flush_icache_all() out of flush_pfn_alias()

ARM: Remove __flush_icache_all() from __flush_dcache_page()
ARM: Avoid flush_dcache_page() for zero page
ARM: Avoid duplicated implementation for VIVT cache flushing
ARM: Allocate ZERO_PAGE from highmem
ARM: Avoid evaluating page_address() multiple times
ARM: Add I-cache invalidation for VIVT ASID tagged caches
ARM: arrange for flush_cache_range() to always flush the I-cache
ARM: avoid flushing I-cache in flush_cache_mm()

for some reason the patches seem not to have made it to the list. I haven't received them in my mailbox, and I can't find them in the archive.

Quoted 19 lines · Russell King

Errata 411920 indicates that any "invalidate entire instruction cache" operation can fail if the right conditions are present. This is not limited just to those operations in flush.c, but elsewhere. Place the workaround in the already existing __flush_icache_all() function instead.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/include/asm/cacheflush.h |    5 +++++
 arch/arm/mm/context.c             |    4 ++--
 arch/arm/mm/flush.c               |   31 ++++++-------------------------
 3 files changed, 13 insertions(+), 27 deletions(-)

arch/arm/include/asm/cacheflush.h +0 -0

index 3147f75..af0624c 100644
@@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
 

arch/arm/include/asm/cacheflush.h extern void flush_dcache_page(struct page *); · line 418

Quoted 6 lines

arch/arm/include/asm/cacheflush.h +4 -0

 static inline void __flush_icache_all(void)
 {
+#ifdef CONFIG_ARM_ERRATA_411920
+	extern void v6_icache_inval_all(void);
+	v6_icache_inval_all();
+#else

I think some compiler complain of "extern" declarations inside a function, not entirely sure.

arch/arm/include/asm/cacheflush.h extern void flush_dcache_page(struct page *); · line 423

Quoted 5 lines

arch/arm/include/asm/cacheflush.h +1 -0

        asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
            :
            : "r" (0));
+#endif
 }

For correctness, can we add a dsb() at the end of this function so that the I-cache operation is guaranteed to complete?

Quoted 7 lines

arch/arm/mm/context.c +0 -0

index 6bda76a..d4f8275 100644
@@ -50,8 +50,8 @@ void __new_context(struct mm_struct *mm)
 		isb();
 		flush_tlb_all();

arch/arm/mm/context.c void __new_context(struct mm_struct *mm) · line 54

Quoted 5 lines

arch/arm/mm/context.c +2 -2

 		if (icache_is_vivt_asid_tagged()) {
-			asm("mcr	p15, 0, %0, c7, c5, 0	@ invalidate I-cache\n"
-			    "mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB\n"
+			__flush_icache_all();
+			asm("mcr	p15, 0, %0, c7, c5, 6	@ flush BTAC/BTB"

IIUC, we don't need to flush the BTAC after a full I-cache invalidation (not sure why I added it).

Quoted 28 lines · Russell King

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mm/flush.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

arch/arm/mm/flush.c +6 -0

index dc66f86..9770e27 100644
@@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 		    : "cc");
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 
 void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)

@@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 		flush_pfn_alias(pfn, user_addr);
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 

arch/arm/mm/flush.c void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig · line 93

Quoted 1 line

arch/arm/mm/flush.c +0 -0

 void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,

You may be right but why is this needed? Between mm's, we have the ASID. Within the same mm, do we expect to change the corresponding physical address of a virtual address without calling update_mmu_cache?

Quoted 7 lines · Russell King

Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache) caused by the dynamic linker changing protection on text pages to write GOT entries. The problem is due to an interaction between the write faulting code providing new anonymous pages (which haven't had their I-cache cleaned due to no VM_EXEC) and change_protection() not doing any kind of I-cache maintainence.

Quoted 4 lines

Work around this by flushing the I-cache in flush_cache_range() for VM_EXEC VMAs (which is what we have when the region is being made executable again.) This ensures that the I-cache will be up to date with the new COW'd pages.

Are you sure this fixes the problem? IIRC this is mainly caused by the write-allocate D-cache and that copy_to_user_highpage() doesn't flush the D-cache. Just an I-cache invalidation wouldn't help much.

Hi Russel,

for some reason the patches seem not to have made it to the list. I haven't received them in my mailbox, and I can't find them in the archive.

Quoted 1 line · Laurent Pinchart

Hi Russel,

Grr.

Quoted 3 lines

for some reason the patches seem not to have made it to the list. I haven't received them in my mailbox, and I can't find them in the archive.

I've no idea either; they got stuck in the moderation queue last night and I assume are still pending.

I'm sure the list admins will eventually get around to it.

Quoted 29 lines · Catalin Marinas

Quoted 28 lines · Russell King

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mm/flush.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

arch/arm/mm/flush.c +6 -0

index dc66f86..9770e27 100644
@@ -69,6 +69,9 @@ void flush_cache_range(struct vm_area_struct *vma, unsigned long start, unsigned
 		    : "cc");
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 
 void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsigned long pfn)

@@ -82,6 +85,9 @@ void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig
 		flush_pfn_alias(pfn, user_addr);
 		__flush_icache_all();
 	}
+
+	if (vma->vm_flags & VM_EXEC && icache_is_vivt_asid_tagged())
+		__flush_icache_all();
 }
 

Quoted 5 lines

arch/arm/mm/flush.c void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr, unsig · line 93

Quoted 1 line

arch/arm/mm/flush.c +0 -0

 void flush_ptrace_access(struct vm_area_struct *vma, struct page *page,

You may be right but why is this needed? Between mm's, we have the ASID. Within the same mm, do we expect to change the corresponding physical address of a virtual address without calling update_mmu_cache?

Stop thinking that update_mmu_cache() has something to do with cache handling. That is not its purpose. update_mmu_cache() is there for systems with software-TLBs to efficiently update the software-TLB for the page fault which has happened.

flush_cache_page() on the other hand is called whenever we change a page mapping - but it's not called when mm's change.

The ARM ARM specifies that when we change the page tables, and we have a VIVT I-cache, it needs to be flushed. The above is merely complying with that statement.

Quoted 13 lines · Catalin Marinas

Quoted 11 lines · Russell King

Dirk Behme reported instability on ARM11 SMP (VIPT non-aliasing cache) caused by the dynamic linker changing protection on text pages to write GOT entries. The problem is due to an interaction between the write faulting code providing new anonymous pages (which haven't had their I-cache cleaned due to no VM_EXEC) and change_protection() not doing any kind of I-cache maintainence.

Work around this by flushing the I-cache in flush_cache_range() for VM_EXEC VMAs (which is what we have when the region is being made executable again.) This ensures that the I-cache will be up to date with the new COW'd pages.

Quoted 3 lines

Are you sure this fixes the problem? IIRC this is mainly caused by the write-allocate D-cache and that copy_to_user_highpage() doesn't flush the D-cache. Just an I-cache invalidation wouldn't help much.

Not on its own (sorry, should've made that clear).

I'm not entirely
sure what's happening with the other half of the fix (the thread seemed to die before reaching any kind of conclusion) but this is the right solution for the I-cache side of the problem.

Quoted 7 lines · Catalin Marinas

Quoted 6 lines · Russell King

arch/arm/include/asm/cacheflush.h +0 -0

index 3147f75..af0624c 100644
@@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
 

Quoted 14 lines

arch/arm/include/asm/cacheflush.h extern void flush_dcache_page(struct page *); · line 423

Quoted 11 lines

arch/arm/include/asm/cacheflush.h +5 -0

 static inline void __flush_icache_all(void)
 {
+#ifdef CONFIG_ARM_ERRATA_411920
+	extern void v6_icache_inval_all(void);
+	v6_icache_inval_all();
+#else
        asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
            :
            : "r" (0));
+#endif
 }

For correctness, can we add a dsb() at the end of this function so that the I-cache operation is guaranteed to complete?

Two reasons why not to in this patch:

1. it changes the produced code, and therefore is another functional change.
2. it means we end up with a sequence like:

	data cache maintainence
	dsb();
	instruction cache maintainence
	dsb();

The dsb() between the two operations probably isn't required.

Quoted 28 lines · Russell King - ARM Linux

Quoted 7 lines · Catalin Marinas

Quoted 6 lines · Russell King

arch/arm/include/asm/cacheflush.h +0 -0

index 3147f75..af0624c 100644
@@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
 

Quoted 14 lines

arch/arm/include/asm/cacheflush.h extern void flush_dcache_page(struct page *); · line 423

Quoted 11 lines

arch/arm/include/asm/cacheflush.h +5 -0

 static inline void __flush_icache_all(void)
 {
+#ifdef CONFIG_ARM_ERRATA_411920
+	extern void v6_icache_inval_all(void);
+	v6_icache_inval_all();
+#else
        asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
            :
            : "r" (0));
+#endif
 }

For correctness, can we add a dsb() at the end of this function so that the I-cache operation is guaranteed to complete?

Two reasons why not to in this patch:

1. it changes the produced code, and therefore is another functional change.
2. it means we end up with a sequence like:

Quoted 6 lines

	data cache maintainence
	dsb();
	instruction cache maintainence
	dsb();

The dsb() between the two operations probably isn't required.

That's described in B2.2.7 in the ARM ARM (make sure you use the latest errata markup - 4.0).

The first DSB is needed to ensure the the D-cache op completed before the I-cache op (in case we get speculative prefetches into the I-cache before the D-cache op completed).

The second DSB would be needed to ensure the completion of the I-cache maintenance operation. An ISB may be needed as well but the return from exception has the same effect, so user code isn't affected.

Quoted 39 lines · Catalin Marinas

Quoted 28 lines · Russell King - ARM Linux

Quoted 7 lines · Catalin Marinas

Quoted 6 lines · Russell King

arch/arm/include/asm/cacheflush.h +0 -0

index 3147f75..af0624c 100644
@@ -412,9 +412,14 @@ extern void flush_dcache_page(struct page *);
 

Quoted 14 lines

arch/arm/include/asm/cacheflush.h extern void flush_dcache_page(struct page *); · line 423

Quoted 11 lines

arch/arm/include/asm/cacheflush.h +5 -0

 static inline void __flush_icache_all(void)
 {
+#ifdef CONFIG_ARM_ERRATA_411920
+	extern void v6_icache_inval_all(void);
+	v6_icache_inval_all();
+#else
        asm("mcr        p15, 0, %0, c7, c5, 0   @ invalidate I-cache\n"
            :
            : "r" (0));
+#endif
 }

For correctness, can we add a dsb() at the end of this function so that the I-cache operation is guaranteed to complete?

Two reasons why not to in this patch:

1. it changes the produced code, and therefore is another functional change.
2. it means we end up with a sequence like:

Quoted 6 lines

	data cache maintainence
	dsb();
	instruction cache maintainence
	dsb();

The dsb() between the two operations probably isn't required.

That's described in B2.2.7 in the ARM ARM (make sure you use the latest errata markup - 4.0).

Quoted 7 lines

The first DSB is needed to ensure the the D-cache op completed before the I-cache op (in case we get speculative prefetches into the I-cache before the D-cache op completed).

The second DSB would be needed to ensure the completion of the I-cache maintenance operation. An ISB may be needed as well but the return from exception has the same effect, so user code isn't affected.

Yes, it looks that way.

However, it looks like a DSB is not required for the work-around case - the I-cache invalidation must complete before any branch prediction occurs, and that's the purpose of the NOPs (to space other code well outside of the visibility of any prefetching which might interfere with the workaround.)

Quoted 2 lines · Laurent Pinchart

for some reason the patches seem not to have made it to the list. I haven't received them in my mailbox, and I can't find them in the archive.

They came as reply without a Re:, so they are probably held in the moderation queue. It would be good if the list checks for [RFC or [PATCH but is for dwmw2 to fix.

flush_cache_mm() is called in two cases:
1. when a process exits, just before the page tables are torn down. We can allow the stale lines to evict themselves over time without causing any harm.

Quoted 4 lines · Russell King

flush_cache_mm() is called in two cases:
1. when a process exits, just before the page tables are torn down. We can allow the stale lines to evict themselves over time without causing any harm.

Is not there any risk to see cache lines evicted at a time when there are no longer any page table entries for them? This was said to be a problem during the early discussions on FCSE:

Quoted 5 lines · Gilles Chanteperdrix

Quoted 4 lines · Russell King

flush_cache_mm() is called in two cases:
1. when a process exits, just before the page tables are torn down. We can allow the stale lines to evict themselves over time without causing any harm.

Quoted 3 lines

Is not there any risk to see cache lines evicted at a time when there are no longer any page table entries for them? This was said to be a problem during the early discussions on FCSE:

The discussion here is about the instruction cache - that doesn't write back cache lines.

Quoted 4 lines · Russell King - ARM Linux

I'm not entirely
sure what's happening with the other half of the fix (the thread seemed to die before reaching any kind of conclusion) but this is the right solution for the I-cache side of the problem.

I listed a few solutions there but not many replies. Let's try to sort it out in this thread (maybe people got bored and ignored the other thread completely :-)).

Flush the D-cache during copy_user_highpage()

From: Catalin Marinas <[email protected]>

The I and D caches for copy-on-write pages on processors with write-allocate caches become incoherent causing problems on application relying on CoW for text pages (dynamic linker relocating symbols in a text page). This patch flushes the D-cache for such pages.

Signed-off-by Catalin Marinas <[email protected]>

---
 arch/arm/mm/copypage-v6.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
 	kfrom = kmap_atomic(from, KM_USER0);
 	kto = kmap_atomic(to, KM_USER1);
 	copy_page(kto, kfrom);

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+	__cpuc_flush_dcache_page(kto);
 	kunmap_atomic(kto, KM_USER1);
 	kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Russell,

Quoted 8 lines · Catalin Marinas

Quoted 4 lines · Russell King - ARM Linux

I'm not entirely
sure what's happening with the other half of the fix (the thread seemed to die before reaching any kind of conclusion) but this is the right solution for the I-cache side of the problem.

I listed a few solutions there but not many replies. Let's try to sort it out in this thread (maybe people got bored and ignored the other thread completely :-)).

[...]

Quoted 14 lines

Flush the D-cache during copy_user_highpage()

From: Catalin Marinas <[email protected]>

The I and D caches for copy-on-write pages on processors with write-allocate caches become incoherent causing problems on application relying on CoW for text pages (dynamic linker relocating symbols in a text page). This patch flushes the D-cache for such pages.

Signed-off-by Catalin Marinas <[email protected]>

---
 arch/arm/mm/copypage-v6.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 12 lines

arch/arm/mm/copypage-v6.c +1 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
 	kfrom = kmap_atomic(from, KM_USER0);
 	kto = kmap_atomic(to, KM_USER1);
 	copy_page(kto, kfrom);
+	__cpuc_flush_dcache_page(kto);
 	kunmap_atomic(kto, KM_USER1);
 	kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Quoted 8 lines · Catalin Marinas

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
 	kfrom = kmap_atomic(from, KM_USER0);
 	kto = kmap_atomic(to, KM_USER1);
 	copy_page(kto, kfrom);

Quoted 8 lines

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+	__cpuc_flush_dcache_page(kto);
 	kunmap_atomic(kto, KM_USER1);
 	kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Well, it would be useful to get some comment from Nicolas since kunmap_atomic() will call this function for highmem pages. We might want to avoid doing the dcache flush thing multiple times for these, but it looks to me like testing PageHighMem(to) leaves us a hole.

Quoted 18 lines · Russell King - ARM Linux

Quoted 8 lines · Catalin Marinas

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
 	kfrom = kmap_atomic(from, KM_USER0);
 	kto = kmap_atomic(to, KM_USER1);
 	copy_page(kto, kfrom);

Quoted 8 lines

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+	__cpuc_flush_dcache_page(kto);
 	kunmap_atomic(kto, KM_USER1);
 	kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Quoted 4 lines

Well, it would be useful to get some comment from Nicolas since kunmap_atomic() will call this function for highmem pages. We might want to avoid doing the dcache flush thing multiple times for these, but it looks to me like testing PageHighMem(to) leaves us a hole.

Better test page_address(to) and skip the flush if it is NULL. That's what we do elsewhere already. If page_addressto) is NULL then there is no long lasting mapping for that page and the cache will be flushed upon kunmap_atomic(). Hence:

#ifdef CONFIG_HIGHMEM
	/*
	 * kmap_atomic() doesn't set the page virtual address, and
	 * kunmap_atomic() takes care of cache flushing already.
	 */
	if (page_address(to) != NULL)
#endif
		__cpuc_flush_dcache_page(kto);

Quoted 30 lines · Nicolas Pitre

Quoted 18 lines · Russell King - ARM Linux

Quoted 8 lines · Catalin Marinas

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
        kfrom = kmap_atomic(from, KM_USER0);
        kto = kmap_atomic(to, KM_USER1);
        copy_page(kto, kfrom);

Quoted 8 lines

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+       __cpuc_flush_dcache_page(kto);
        kunmap_atomic(kto, KM_USER1);
        kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Quoted 4 lines

Well, it would be useful to get some comment from Nicolas since kunmap_atomic() will call this function for highmem pages. We might want to avoid doing the dcache flush thing multiple times for these, but it looks to me like testing PageHighMem(to) leaves us a hole.

Better test page_address(to) and skip the flush if it is NULL. That's what we do elsewhere already. If page_addressto) is NULL then there is no long lasting mapping for that page and the cache will be flushed upon kunmap_atomic(). Hence:

Quoted 8 lines

#ifdef CONFIG_HIGHMEM
      /*
       * kmap_atomic() doesn't set the page virtual address, and
       * kunmap_atomic() takes care of cache flushing already.
       */
      if (page_address(to) != NULL)
#endif
              __cpuc_flush_dcache_page(kto);

Okay, can we get this combined with Catalin's patch and applied to all applicable processors (which I assume would be just v6 and v7) ?

Quoted 40 lines · Russell King - ARM Linux

Quoted 38 lines · Nicolas Pitre

Quoted 18 lines · Russell King - ARM Linux

Quoted 8 lines · Catalin Marinas

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
        kfrom = kmap_atomic(from, KM_USER0);
        kto = kmap_atomic(to, KM_USER1);
        copy_page(kto, kfrom);

Quoted 8 lines

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+       __cpuc_flush_dcache_page(kto);
        kunmap_atomic(kto, KM_USER1);
        kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Quoted 4 lines

Well, it would be useful to get some comment from Nicolas since kunmap_atomic() will call this function for highmem pages. We might want to avoid doing the dcache flush thing multiple times for these, but it looks to me like testing PageHighMem(to) leaves us a hole.

Better test page_address(to) and skip the flush if it is NULL. That's what we do elsewhere already. If page_addressto) is NULL then there is no long lasting mapping for that page and the cache will be flushed upon kunmap_atomic(). Hence:

#ifdef CONFIG_HIGHMEM
      /*
       * kmap_atomic() doesn't set the page virtual address, and
       * kunmap_atomic() takes care of cache flushing already.
       */
      if (page_address(to) != NULL)
#endif
              __cpuc_flush_dcache_page(kto);

Quoted 2 lines

Okay, can we get this combined with Catalin's patch and applied to all applicable processors (which I assume would be just v6 and v7) ?

I'm ok with this.

Quoted 44 lines · Catalin Marinas

Quoted 42 lines · Russell King - ARM Linux

Quoted 30 lines · Nicolas Pitre

Quoted 18 lines · Russell King - ARM Linux

Quoted 8 lines · Catalin Marinas

Quoted 8 lines

arch/arm/mm/copypage-v6.c +0 -0

index 4127a7b..f19ed4e 100644
@@ -41,6 +41,7 @@ static void v6_copy_user_highpage_nonaliasing(struct page *to,
        kfrom = kmap_atomic(from, KM_USER0);
        kto = kmap_atomic(to, KM_USER1);
        copy_page(kto, kfrom);

Quoted 8 lines

arch/arm/mm/copypage-v6.c static void v6_copy_user_highpage_nonaliasing(struct page *to, · line 47

Quoted 4 lines

arch/arm/mm/copypage-v6.c +1 -0

+       __cpuc_flush_dcache_page(kto);
        kunmap_atomic(kto, KM_USER1);
        kunmap_atomic(kfrom, KM_USER0);
 }

Any thoughts one getting this merged (maybe in 2.6.33)? It seems that no-one else has any views on this but the COW text pages problem is still present.

Quoted 4 lines

Well, it would be useful to get some comment from Nicolas since kunmap_atomic() will call this function for highmem pages. We might want to avoid doing the dcache flush thing multiple times for these, but it looks to me like testing PageHighMem(to) leaves us a hole.

Better test page_address(to) and skip the flush if it is NULL. That's what we do elsewhere already. If page_addressto) is NULL then there is no long lasting mapping for that page and the cache will be flushed upon kunmap_atomic(). Hence:

Quoted 8 lines

#ifdef CONFIG_HIGHMEM
      /*
       * kmap_atomic() doesn't set the page virtual address, and
       * kunmap_atomic() takes care of cache flushing already.
       */
      if (page_address(to) != NULL)
#endif
              __cpuc_flush_dcache_page(kto);

Okay, can we get this combined with Catalin's patch and applied to all applicable processors (which I assume would be just v6 and v7) ?

Quoted 1 line

I'm ok with this.

Sorry, I wasn't clear enough. I was expecting you to update your patch, and once the other copy_user_page stuff finally gets the "yes it solves the problem" response from Steven, it can be committed and merged into this branch.

OK, no problem. I thought you have a bigger pile of patches and just squeeze this change somewhere. I'll post a new patch.

The zero page does not need to be in low memory, so allocate it from high memory.