[PATCH 0/4][RFC] Trivial ARM related Android patches

[PATCH 0/4][RFC] Trivial ARM related Android patches

[John Stultz]

So after all the heat that was generated in the various Android
discussions, I took a look a look at the android git tree, and 
while there are a fair number of large and controversial 
infrastructure changes, there are also a number of small fixes 
that apply easily against Linus' git tree.

So after cherry picking these 50-some small patches out of the
android tree, I organized them into topic branches, and over
the next few weeks, I hope to send them out to lkml and topic
maintainers for comments.

Now, I'm not proposing that these changes be merged as-is. It
may very well be that, unknown to me, android developers have
already tried to submit these patches and they have been rejected
for good reason. Or some patches may very well be necessary hacks
to get thing shipping while deeper fixes are being worked on. If
that is the case, let me know and forgive me for the noise.

But as, it seemed many of these small changes have been obscured
by the debate over the larger infrastructure changes, I wanted 
to bring them forward so that possibly good fixes were not missed
in the controversy.

Maintainers: If you do find any of these patches distasteful,
that's fine, I'll be happy to drop them from my tree for now.
I really don't want to stir up another huge mail thread over these 
small patches, but I'd appreciate if you'd consider them as a
bug report illustrating an issue or a desired feature, and suggest 
what you see as a reasonable way to accomplish the desired
functionality presented in the patch.

The following patches are just the ARM related trivial patches
from the Android tree. You can find these as well as my other
Android topic branches here:
http://git.linaro.org/gitweb?p=people/jstultz/linux.git;a=summary

thanks
-john

CC: Arve Hj?nnev?g <arve@android.com>
CC: Brian Swetland <swetland@google.com>
CC: Dima Zavin <dima@android.com>
CC: San Mehat <san@google.com>
CC: Michael Davidson <md@google.com>
CC: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Russell King <linux@arm.linux.org.uk>


Arve Hj??nnev??g (1):
  Optionally flush entire dcache from v6_dma_flush_range

Brian Swetland (1):
  avoid mis-detecting some V7 cores in the decompressor

Dima Zavin (1):
  Do not call flush_cache_user_range with mmap_sem held

San Mehat (1):
  process: Add display of memory around registers when displaying regs.

 arch/arm/boot/compressed/head.S   |    3 ++
 arch/arm/include/asm/cacheflush.h |    2 +-
 arch/arm/kernel/process.c         |   73 +++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/traps.c           |    4 ++-
 arch/arm/mm/cache-v6.S            |   17 +++++++++
 5 files changed, 97 insertions(+), 2 deletions(-)

-- 
1.7.3.2.146.gca209

[PATCH 1/4] avoid mis-detecting some V7 cores in the decompressor

[John Stultz]

From: Brian Swetland <swetland@google.com>

This allows kernel decompress to happen nearly instantly instead
of taking over 20 seconds.

CC: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Brian Swetland <swetland@google.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm/boot/compressed/head.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 6825c34..18d1847 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -637,6 +637,8 @@ proc_types:
 @		b	__arm6_mmu_cache_off
 @		b	__armv3_mmu_cache_flush
 
+#if !defined(CONFIG_CPU_V7)
+		/* This collides with some V7 IDs, preventing correct detection */
 		.word	0x00000000		@ old ARM ID
 		.word	0x0000f000
 		mov	pc, lr
@@ -645,6 +647,7 @@ proc_types:
  THUMB(		nop				)
 		mov	pc, lr
  THUMB(		nop				)
+#endif
 
 		.word	0x41007000		@ ARM7/710
 		.word	0xfff8fe00
-- 
1.7.3.2.146.gca209

[PATCH 2/4] Optionally flush entire dcache from v6_dma_flush_range

[John Stultz]

From: Arve Hj?nnev?g <arve@android.com>

If CACHE_FLUSH_RANGE_LIMIT is defined, then the entire dcache will
be flushed if the requested range is larger than this limit.

CC: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Russell King <linux@arm.linux.org.uk>
Change-Id: I29277d645a9d6716b1952cf3b870c78496261dd0
Signed-off-by: Arve Hj?nnev?g <arve@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm/mm/cache-v6.S |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
index 99fa688..92c66b4 100644
--- a/arch/arm/mm/cache-v6.S
+++ b/arch/arm/mm/cache-v6.S
@@ -263,6 +263,11 @@ v6_dma_clean_range:
  *	- end     - virtual end address of region
  */
 ENTRY(v6_dma_flush_range)
+#ifdef CONFIG_CACHE_FLUSH_RANGE_LIMIT
+	sub	r2, r1, r0
+	cmp	r2, #CONFIG_CACHE_FLUSH_RANGE_LIMIT
+	bhi	v6_dma_flush_dcache_all
+#endif
 	bic	r0, r0, #D_CACHE_LINE_SIZE - 1
 1:
 #ifdef CONFIG_DMA_CACHE_RWFO
@@ -281,6 +286,18 @@ ENTRY(v6_dma_flush_range)
 	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
 	mov	pc, lr
 
+#ifdef CONFIG_CACHE_FLUSH_RANGE_LIMIT
+v6_dma_flush_dcache_all:
+	mov	r0, #0
+#ifdef HARVARD_CACHE
+	mcr	p15, 0, r0, c7, c14, 0		@ D cache clean+invalidate
+#else
+	mcr	p15, 0, r0, c7, c15, 0		@ Cache clean+invalidate
+#endif
+	mcr	p15, 0, r0, c7, c10, 4		@ drain write buffer
+	mov	pc, lr
+#endif
+
 /*
  *	dma_map_area(start, size, dir)
  *	- start	- kernel virtual start address
-- 
1.7.3.2.146.gca209

[PATCH 3/4] process: Add display of memory around registers when displaying regs.

[John Stultz]

From: San Mehat <san@google.com>

This is extremely useful in diagnosing remote crashes, and is based heavily
on original work by <md@google.com>.

Signed-off-by: San Mehat <san@google.com>
Cc: Michael Davidson <md@google.com>

[ARM] process: Use uber-safe probe_kernel_address() to read mem when dumping.

This prevents the dump from taking pagefaults / external aborts.

CC: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Russell King <linux@arm.linux.org.uk>
Signed-off-by: San Mehat <san@google.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm/kernel/process.c |   73 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index e76fcaa..82c37fe 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -251,6 +251,77 @@ void machine_restart(char *cmd)
 	arm_pm_restart(reboot_mode, cmd);
 }
 
+/*
+ * dump a block of kernel memory from around the given address
+ */
+static void show_data(unsigned long addr, int nbytes, const char *name)
+{
+	int	i, j;
+	int	nlines;
+	u32	*p;
+
+	/*
+	 * don't attempt to dump non-kernel addresses or
+	 * values that are probably just small negative numbers
+	 */
+	if (addr < PAGE_OFFSET || addr > -256UL)
+		return;
+
+	printk("\n%s: %#lx:\n", name, addr);
+
+	/*
+	 * round address down to a 32 bit boundary
+	 * and always dump a multiple of 32 bytes
+	 */
+	p = (u32 *)(addr & ~(sizeof(u32) - 1));
+	nbytes += (addr & (sizeof(u32) - 1));
+	nlines = (nbytes + 31) / 32;
+
+
+	for (i = 0; i < nlines; i++) {
+		/*
+		 * just display low 16 bits of address to keep
+		 * each line of the dump < 80 characters
+		 */
+		printk("%04lx ", (unsigned long)p & 0xffff);
+		for (j = 0; j < 8; j++) {
+			u32	data;
+			if (probe_kernel_address(p, data)) {
+				printk(" ********");
+			} else {
+				printk(" %08x", data);
+			}
+			++p;
+		}
+		printk("\n");
+	}
+}
+
+static void show_extra_register_data(struct pt_regs *regs, int nbytes)
+{
+	mm_segment_t fs;
+
+	fs = get_fs();
+	set_fs(KERNEL_DS);
+	show_data(regs->ARM_pc - nbytes, nbytes * 2, "PC");
+	show_data(regs->ARM_lr - nbytes, nbytes * 2, "LR");
+	show_data(regs->ARM_sp - nbytes, nbytes * 2, "SP");
+	show_data(regs->ARM_ip - nbytes, nbytes * 2, "IP");
+	show_data(regs->ARM_fp - nbytes, nbytes * 2, "FP");
+	show_data(regs->ARM_r0 - nbytes, nbytes * 2, "R0");
+	show_data(regs->ARM_r1 - nbytes, nbytes * 2, "R1");
+	show_data(regs->ARM_r2 - nbytes, nbytes * 2, "R2");
+	show_data(regs->ARM_r3 - nbytes, nbytes * 2, "R3");
+	show_data(regs->ARM_r4 - nbytes, nbytes * 2, "R4");
+	show_data(regs->ARM_r5 - nbytes, nbytes * 2, "R5");
+	show_data(regs->ARM_r6 - nbytes, nbytes * 2, "R6");
+	show_data(regs->ARM_r7 - nbytes, nbytes * 2, "R7");
+	show_data(regs->ARM_r8 - nbytes, nbytes * 2, "R8");
+	show_data(regs->ARM_r9 - nbytes, nbytes * 2, "R9");
+	show_data(regs->ARM_r10 - nbytes, nbytes * 2, "R10");
+	set_fs(fs);
+}
+
 void __show_regs(struct pt_regs *regs)
 {
 	unsigned long flags;
@@ -310,6 +381,8 @@ void __show_regs(struct pt_regs *regs)
 		printk("Control: %08x%s\n", ctrl, buf);
 	}
 #endif
+
+	show_extra_register_data(regs, 128);
 }
 
 void show_regs(struct pt_regs * regs)
-- 
1.7.3.2.146.gca209

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[John Stultz]

From: Dima Zavin <dima@android.com>

We can't be holding the mmap_sem while calling flush_cache_user_range
because the flush can fault. If we fault on a user address, the
page fault handler will try to take mmap_sem again. Since both places
acquire the read lock, most of the time it succeeds. However, if another
thread tries to acquire the write lock on the mmap_sem (e.g. mmap) in
between the call to flush_cache_user_range and the fault, the down_read
in do_page_fault will deadlock.

Also, since we really can't be holding the mmap_sem while calling
flush_cache_user_range AND vma is actually unused by the flush itself,
get rid of vma as an argument.

CC: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Dima Zavin <dima@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 arch/arm/include/asm/cacheflush.h |    2 +-
 arch/arm/kernel/traps.c           |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h
index 3acd8fa..f533a6a 100644
--- a/arch/arm/include/asm/cacheflush.h
+++ b/arch/arm/include/asm/cacheflush.h
@@ -377,7 +377,7 @@ extern void flush_cache_page(struct vm_area_struct *vma, unsigned long user_addr
  * Harvard caches are synchronised for the user space address range.
  * This is used for the ARM private sys_cacheflush system call.
  */
-#define flush_cache_user_range(vma,start,end) \
+#define flush_cache_user_range(start,end) \
 	__cpuc_coherent_user_range((start) & PAGE_MASK, PAGE_ALIGN(end))
 
 /*
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 446aee9..7644a56 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -451,7 +451,9 @@ do_cache_op(unsigned long start, unsigned long end, int flags)
 		if (end > vma->vm_end)
 			end = vma->vm_end;
 
-		flush_cache_user_range(vma, start, end);
+		up_read(&mm->mmap_sem);
+		flush_cache_user_range(start, end);
+		return;
 	}
 	up_read(&mm->mmap_sem);
 }
-- 
1.7.3.2.146.gca209

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Russell King - ARM Linux]

On Mon, Dec 13, 2010 at 08:57:39PM -0800, John Stultz wrote:
> From: Dima Zavin <dima@android.com>
> 
> We can't be holding the mmap_sem while calling flush_cache_user_range
> because the flush can fault. If we fault on a user address, the
> page fault handler will try to take mmap_sem again. Since both places
> acquire the read lock, most of the time it succeeds. However, if another
> thread tries to acquire the write lock on the mmap_sem (e.g. mmap) in
> between the call to flush_cache_user_range and the fault, the down_read
> in do_page_fault will deadlock.
> 
> Also, since we really can't be holding the mmap_sem while calling
> flush_cache_user_range AND vma is actually unused by the flush itself,
> get rid of vma as an argument.

Holding the mmap sem prevents a concurrent munmap, mremap or other change
to the VMA while the flush operation is in progress.  There is no other
lock which will do this for us.

Well, it looks like we can't do this flushing of userspace safely... so
I suggest we disable this interface, thereby preventing userspace from
a whole host of actions requiring cache maintainence.  Unless someone can
come up with a better fix.

[PATCH 2/4] Optionally flush entire dcache from v6_dma_flush_range

[Russell King - ARM Linux]

On Mon, Dec 13, 2010 at 08:57:37PM -0800, John Stultz wrote:
> From: Arve Hj?nnev?g <arve@android.com>
> 
> If CACHE_FLUSH_RANGE_LIMIT is defined, then the entire dcache will
> be flushed if the requested range is larger than this limit.

This has been talked about before, and it's not the correct place to do
this.  With a scatterlist, you'll end up repeatedly flushing the cache.

[PATCH 3/4] process: Add display of memory around registers when displaying regs.

[Russell King - ARM Linux]

On Mon, Dec 13, 2010 at 08:57:38PM -0800, John Stultz wrote:
> From: San Mehat <san@google.com>
> 
> This is extremely useful in diagnosing remote crashes, and is based heavily
> on original work by <md@google.com>.

Why reinvent dump_mem() ?

If we dump out two lines of data per register, that's 32 additional
lines in the oops dump.  That might make it difficult to get the
register information which may have been lost off the scrollback.

Or worse still, it might wrap the kernel buffers before syslog gets
it.

[PATCH 2/4] Optionally flush entire dcache from v6_dma_flush_range

[Catalin Marinas]

On 14 December 2010 04:57, John Stultz <john.stultz@linaro.org> wrote:
> From: Arve Hj?nnev?g <arve@android.com>
>
> If CACHE_FLUSH_RANGE_LIMIT is defined, then the entire dcache will
> be flushed if the requested range is larger than this limit.
>
> CC: Nicolas Pitre <nicolas.pitre@linaro.org>
> CC: Russell King <linux@arm.linux.org.uk>
> Change-Id: I29277d645a9d6716b1952cf3b870c78496261dd0
> Signed-off-by: Arve Hj?nnev?g <arve@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> ?arch/arm/mm/cache-v6.S | ? 17 +++++++++++++++++
> ?1 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/cache-v6.S b/arch/arm/mm/cache-v6.S
> index 99fa688..92c66b4 100644
> --- a/arch/arm/mm/cache-v6.S
> +++ b/arch/arm/mm/cache-v6.S
> @@ -263,6 +263,11 @@ v6_dma_clean_range:
> ?* ? ? - end ? ? - virtual end address of region
> ?*/
> ?ENTRY(v6_dma_flush_range)
> +#ifdef CONFIG_CACHE_FLUSH_RANGE_LIMIT
> + ? ? ? sub ? ? r2, r1, r0
> + ? ? ? cmp ? ? r2, #CONFIG_CACHE_FLUSH_RANGE_LIMIT
> + ? ? ? bhi ? ? v6_dma_flush_dcache_all
> +#endif
> ? ? ? ?bic ? ? r0, r0, #D_CACHE_LINE_SIZE - 1
> ?1:
> ?#ifdef CONFIG_DMA_CACHE_RWFO

I'm not sure how CONFIG_CACHE_FLUSH_RANGE_LIMIT is defined but it
should depend on !CONFIG_DMA_CACHE_RWFO or just add this check to the
#ifdef. The D-cache flushing ops aren't broadcast on ARM11MPCore.

-- 
Catalin

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

On 14 December 2010 09:30, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Dec 13, 2010 at 08:57:39PM -0800, John Stultz wrote:
>> From: Dima Zavin <dima@android.com>
>>
>> We can't be holding the mmap_sem while calling flush_cache_user_range
>> because the flush can fault. If we fault on a user address, the
>> page fault handler will try to take mmap_sem again. Since both places
>> acquire the read lock, most of the time it succeeds. However, if another
>> thread tries to acquire the write lock on the mmap_sem (e.g. mmap) in
>> between the call to flush_cache_user_range and the fault, the down_read
>> in do_page_fault will deadlock.
>>
>> Also, since we really can't be holding the mmap_sem while calling
>> flush_cache_user_range AND vma is actually unused by the flush itself,
>> get rid of vma as an argument.
>
> Holding the mmap sem prevents a concurrent munmap, mremap or other change
> to the VMA while the flush operation is in progress. ?There is no other
> lock which will do this for us.

But what's the problem if such mapping disappears? The
flush_cache_user_range code should just skip such pages.

-- 
Catalin

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

On 14 December 2010 04:57, John Stultz <john.stultz@linaro.org> wrote:
> From: Dima Zavin <dima@android.com>
>
> We can't be holding the mmap_sem while calling flush_cache_user_range
> because the flush can fault. If we fault on a user address, the
> page fault handler will try to take mmap_sem again. Since both places
> acquire the read lock, most of the time it succeeds. However, if another
> thread tries to acquire the write lock on the mmap_sem (e.g. mmap) in
> between the call to flush_cache_user_range and the fault, the down_read
> in do_page_fault will deadlock.
>
> Also, since we really can't be holding the mmap_sem while calling
> flush_cache_user_range AND vma is actually unused by the flush itself,
> get rid of vma as an argument.
>
> CC: Nicolas Pitre <nicolas.pitre@linaro.org>
> CC: Russell King <linux@arm.linux.org.uk>
> Signed-off-by: Dima Zavin <dima@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Russell King - ARM Linux]

On Tue, Dec 14, 2010 at 05:51:36PM +0000, Catalin Marinas wrote:
> But what's the problem if such mapping disappears? The
> flush_cache_user_range code should just skip such pages.

That's only half the story.

What if someone remaps something over that range before the cache
maintainence has completed.

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

On Tuesday, 14 December 2010, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Dec 14, 2010 at 05:51:36PM +0000, Catalin Marinas wrote:
>> But what's the problem if such mapping disappears? The
>> flush_cache_user_range code should just skip such pages.
>
> That's only half the story.
>
> What if someone remaps something over that range before the cache
> maintainence has completed.

That someone remapping the same range can only be a thread of the same
process. If the code was so badly written as to unmap ranges of memory
when a thread actively uses it, then it probably deserves any
corruption.

OTOH, the cache flushing operations is pretty harmless even if you do
it on the wrong memory range. On ARM11MPCore we probably need to do
some read/write for ownership as in the DMA ops.

There are architectures at allow cache flushing from user space (no
invalidation as that's a bit dangerous) and they don't have any access
to the kernel semaphores. I don't see this any difference here, we
just need to make sure that the kernel can cope with invalid ranges.

-- 
Catalin

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

Russell,

On 14 December 2010 21:08, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tuesday, 14 December 2010, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Tue, Dec 14, 2010 at 05:51:36PM +0000, Catalin Marinas wrote:
>>> But what's the problem if such mapping disappears? The
>>> flush_cache_user_range code should just skip such pages.
>>
>> That's only half the story.
>>
>> What if someone remaps something over that range before the cache
>> maintainence has completed.
>
> That someone remapping the same range can only be a thread of the same
> process. If the code was so badly written as to unmap ranges of memory
> when a thread actively uses it, then it probably deserves any
> corruption.

I haven't seen any more replies in this thread but I still think it's
a real problem as John reported. Do you agree with the original patch?
That's the original post if you no longer have it in your inbox:

http://article.gmane.org/gmane.linux.ports.arm.kernel/99356

-- 
Catalin

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Russell King - ARM Linux]

On Mon, Apr 04, 2011 at 02:27:45PM +0100, Catalin Marinas wrote:
> Russell,
> 
> On 14 December 2010 21:08, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > On Tuesday, 14 December 2010, Russell King - ARM Linux
> > <linux@arm.linux.org.uk> wrote:
> >> On Tue, Dec 14, 2010 at 05:51:36PM +0000, Catalin Marinas wrote:
> >>> But what's the problem if such mapping disappears? The
> >>> flush_cache_user_range code should just skip such pages.
> >>
> >> That's only half the story.
> >>
> >> What if someone remaps something over that range before the cache
> >> maintainence has completed.
> >
> > That someone remapping the same range can only be a thread of the same
> > process. If the code was so badly written as to unmap ranges of memory
> > when a thread actively uses it, then it probably deserves any
> > corruption.
> 
> I haven't seen any more replies in this thread but I still think it's
> a real problem as John reported. Do you agree with the original patch?

No, I do not agree.

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

On Mon, 2011-04-04 at 14:37 +0100, Russell King - ARM Linux wrote:
> On Mon, Apr 04, 2011 at 02:27:45PM +0100, Catalin Marinas wrote:
> > On 14 December 2010 21:08, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > On Tuesday, 14 December 2010, Russell King - ARM Linux
> > > <linux@arm.linux.org.uk> wrote:
> > >> On Tue, Dec 14, 2010 at 05:51:36PM +0000, Catalin Marinas wrote:
> > >>> But what's the problem if such mapping disappears? The
> > >>> flush_cache_user_range code should just skip such pages.
> > >>
> > >> That's only half the story.
> > >>
> > >> What if someone remaps something over that range before the cache
> > >> maintainence has completed.
> > >
> > > That someone remapping the same range can only be a thread of the same
> > > process. If the code was so badly written as to unmap ranges of memory
> > > when a thread actively uses it, then it probably deserves any
> > > corruption.
> >
> > I haven't seen any more replies in this thread but I still think it's
> > a real problem as John reported. Do you agree with the original patch?
> 
> No, I do not agree.

You already stated that and I replied with some arguments. Is this any
different from user cache maintenance available in other architectures
(where user space can't acquire the mmap_sem)?

-- 
Catalin

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Jiejing.Zhang]

Hi Catalin, Russell,

I met this issue on our development board: iMX53 + 2.6.35.2 Kernel,
this is stack trace when the dead lock happens.

You can see the hung task are dead lock at __read_lock(), after I add
more debug message and dump the hung task's stack trace.

After apply this patch, the hung is gong.

This is the hung task's output:

Freezing of tasks failed after 20.01 seconds (7 tasks refusing to freeze):

InputDispatch D 80477548     0  2209   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<8047949c>] (__down_read+0x12c/0x140)
[<8047949c>] (__down_read+0x12c/0x140) from [<800b62a0>]
access_process_vm+0x3c/0x120)
[<800b62a0>] (access_process_vm+0x3c/0x120) from [<8010e528>]
proc_pid_cmdline+0x50/0xcc)
[<8010e528>] (proc_pid_cmdline+0x50/0xcc) from [<8010fafc>]
(proc_info_read+0x58/0xc4)
[<8010fafc>] (proc_info_read+0x58/0xc4) from [<800ce550>] (vfs_read+0xa8/0x150)
[<800ce550>] (vfs_read+0xa8/0x150) from [<800ce6a4>] (sys_read+0x3c/0x68)
[<800ce6a4>] (sys_read+0x3c/0x68) from [<8003af80>] (ret_fast_syscall+0x0/0x30)

android.brows D 80477548     0  2575   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<80479354>]
__down_write_nested+0x128/0x13c)
[<80479354>] (__down_write_nested+0x128/0x13c) from [<800bc62c>]
(sys_mprotect+0xac/0x1f0)
[<800bc62c>] (sys_mprotect+0xac/0x1f0) from [<8003af80>]
(ret_fast_syscall+0x0/0x30)

Binder Thread D 80477548     0  2585   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from
[<80479354>](__down_write_nested+0x128/0x13c)
[<80479354>] (__down_write_nested+0x128/0x13c) from [<800bc0d0>]
(sys_f+0x70/0xb8)
[<800bc0d0>] (sys_mmap_pgoff+0x70/0xb8) from [<8003af80>]
(ret_fast_syscall+0x0/0x30)

WebViewCoreTh D 80477548     0  2606   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<8047949c>] (__down_read+0x12c/0x140)
[<8047949c>] (__down_read+0x12c/0x140) from [<80040494>]
(do_page_fault+0x8c/0x1d4)
[<80040494>] (do_page_fault+0x8c/0x1d4) from [<8003a2c8>]
(do_DataAbort+0x34/0x94)
[<8003a2c8>] (do_DataAbort+0x34/0x94) from [<8003aa2c>] (__dabt_svc+0x4c/0x60)
Exception stack(0xdfffdec0 to 0xdfffdf08)
dec0: 3ace1000 3ace3000 00000040 0000003f 3ace2020 d4323db4 3ace0020 d4323d80
dee0: 00000000 dfffc000 fffffa30 68888714 d4444528 dfffdf08 8003e760 800430e8
df00: 80000013 ffffffff
[<8003aa2c>] (__dabt_svc+0x4c/0x60) from [<800430e8>]
(v7_coherent_kern_range+0x18/0x54)
[<800430e8>] (v7_coherent_kern_range+0x18/0x54) from [<8003e760>]
(arm_syscall+0x14c/0x230)
[<8003e760>] (arm_syscall+0x14c/0x230) from [<8003af80>]
(ret_fast_syscall+0x0/0x30)

WebViewCoreTh D 80477548     0  2610   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<80479354>]
(__down_write_nested+0x128/0x13c)
[<80479354>] (__down_write_nested+0x128/0x13c) from [<800bc62c>]
(sys_mprotect+0xac/0x1f0)
[<800bc62c>] (sys_mprotect+0xac/0x1f0) from [<8003af80>]
(ret_fast_syscall+0x0/0x30)

WebViewCoreTh D 80477548     0  2612   2108 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<8047949c>] (__down_read+0x12c/0x140)
[<8047949c>] (__down_read+0x12c/0x140) from [<80040494>]
(do_page_fault+0x8c/0x1d4)
[<80040494>] (do_page_fault+0x8c/0x1d4) from [<8003a234>]
(do_PrefetchAbort+0x34/0x94)
[<8003a234>] (do_PrefetchAbort+0x34/0x94) from [<8003af20>]
(ret_from_exception+0x0/0x10)
Exception stack(0xdfff3fb0 to 0xdfff3ff8)
3fa0:                                     39f46b18 39f46b18 6833ee01 39f46b18
3fc0: 39f46b18 001f7e20 001e2bd8 39f46b18 39f46b28 39f46b30 39f46b10 39f46b60
3fe0: 00000000 39f46af0 685d27b1 6833ee00 60000030 ffffffff
ps            D 80477548     0  2625   2623 0x00080001
[<80477548>] (schedule+0x2b0/0x334) from [<8047949c>] (__down_read+0x12c/0x140)
[<8047949c>] (__down_read+0x12c/0x140) from [<800b62a0>]
(access_process_vm+0x3c/0x120)
[<800b62a0>] (access_process_vm+0x3c/0x120) from [<8010e528>]
(proc_pid_cmdline+0x50/0xcc)
[<8010e528>] (proc_pid_cmdline+0x50/0xcc) from [<8010fafc>]
(proc_info_read+0x58/0xc4)
[<8010fafc>] (proc_info_read+0x58/0xc4) from [<800ce550>] (vfs_read+0xa8/0x150)
[<800ce550>] (vfs_read+0xa8/0x150) from [<800ce6a4>] (sys_read+0x3c/0x68)
[<800ce6a4>] (sys_read+0x3c/0x68) from [<8003af80>] (ret_fast_syscall+0x0/0x30)

Restarting tasks ... done.
suspend: exit suspend, ret = -16 (2000-01-01 00:48:46.107784876 UTC)
request_suspend_state: on (3->0) at 174620235627 (2000-01-01
00:48:46.127113876 UTC)
mxc_ipu mxc_ipu: Channel already disabled 9
mxc_ipu mxc_ipu: Channel already uninitialized 9
XXXX 1024 768 9997 910313298 960 80 20 6 10 16





Best regards,
Zhang Jiejing

[PATCH 4/4] Do not call flush_cache_user_range with mmap_sem held

[Catalin Marinas]

On Fri, Aug 26, 2011 at 08:32:28AM +0100, Jiejing.Zhang  wrote:
> I met this issue on our development board: iMX53 + 2.6.35.2 Kernel,
> this is stack trace when the dead lock happens.
> 
> You can see the hung task are dead lock at __read_lock(), after I add
> more debug message and dump the hung task's stack trace.
> 
> After apply this patch, the hung is gong.

Russell doesn't agree with the original patch, so we have to live with
this dead-lock in mainline :(. From my perspective, it is a perfectly
valid patch and there is no race. I'll keep it in my trees for now.

If a user space app is so badly written that it calls the cache flushing
function while remapping the same memory range from a different thread,
it deserve some failures. But even then, the cache flushing function is
simply cleaning the D-cache which is a safe operation (as compared to
invalidation).

For reference, that's the original post:

http://article.gmane.org/gmane.linux.ports.arm.kernel/99356

-- 
Catalin