[PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[Ard Biesheuvel]

Commit 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI
unwinder") removed the dummy version of return_address() that was
provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
removal of the kernel_text_address() call from unwind_frame() in the
preceding patch made it safe to do so.

However, this turns out not to be the case: Corentin reports warnings
about suspicious RCU usage and other strange behavior that seems to
originate in the stack unwinding that occurs in return_address().

Given that the function graph tracer (which is what these changes were
enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
this distinction, let's revert return_address() to the old state.

Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
Fixes: 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/include/asm/ftrace.h | 18 ++++++++++++++++++
 arch/arm/kernel/Makefile      |  5 ++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index 5358aad67831..7e9251ca29fe 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -35,8 +35,26 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 
 #ifndef __ASSEMBLY__
 
+#if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND)
+/*
+ * return_address uses walk_stackframe to do it's work.  If both
+ * CONFIG_FRAME_POINTER=y and CONFIG_ARM_UNWIND=y walk_stackframe uses unwind
+ * information.  For this to work in the function tracer many functions would
+ * have to be marked with __notrace.  So for now just depend on
+ * !CONFIG_ARM_UNWIND.
+ */
+
 void *return_address(unsigned int);
 
+#else
+
+static inline void *return_address(unsigned int level)
+{
+       return NULL;
+}
+
+#endif
+
 #define ftrace_return_address(n) return_address(n)
 
 #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index 5cebb8d5a1d6..56511856ff9d 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -25,7 +25,10 @@ obj-y		:= elf.o entry-common.o irq.o opcodes.o \
 KASAN_SANITIZE_stacktrace.o := n
 KASAN_SANITIZE_traps.o := n
 
-obj-y				+= return_address.o
+ifneq ($(CONFIG_ARM_UNWIND),y)
+obj-$(CONFIG_FRAME_POINTER)	+= return_address.o
+endif
+
 obj-$(CONFIG_ATAGS)		+= atags_parse.o
 obj-$(CONFIG_ATAGS_PROC)	+= atags_proc.o
 obj-$(CONFIG_DEPRECATED_PARAM_STRUCT) += atags_compat.o
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[Corentin Labbe]

Le Wed, Mar 02, 2022 at 12:32:01PM +0100, Ard Biesheuvel a écrit :
> Commit 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI
> unwinder") removed the dummy version of return_address() that was
> provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
> removal of the kernel_text_address() call from unwind_frame() in the
> preceding patch made it safe to do so.
> 
> However, this turns out not to be the case: Corentin reports warnings
> about suspicious RCU usage and other strange behavior that seems to
> originate in the stack unwinding that occurs in return_address().
> 
> Given that the function graph tracer (which is what these changes were
> enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
> this distinction, let's revert return_address() to the old state.
> 
> Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
> Fixes: 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm/include/asm/ftrace.h | 18 ++++++++++++++++++
>  arch/arm/kernel/Makefile      |  5 ++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index 5358aad67831..7e9251ca29fe 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -35,8 +35,26 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  
>  #ifndef __ASSEMBLY__
>  
> +#if defined(CONFIG_FRAME_POINTER) && !defined(CONFIG_ARM_UNWIND)
> +/*
> + * return_address uses walk_stackframe to do it's work.  If both
> + * CONFIG_FRAME_POINTER=y and CONFIG_ARM_UNWIND=y walk_stackframe uses unwind
> + * information.  For this to work in the function tracer many functions would
> + * have to be marked with __notrace.  So for now just depend on
> + * !CONFIG_ARM_UNWIND.
> + */
> +
>  void *return_address(unsigned int);
>  
> +#else
> +
> +static inline void *return_address(unsigned int level)
> +{
> +       return NULL;
> +}
> +
> +#endif
> +
>  #define ftrace_return_address(n) return_address(n)
>  
>  #define ARCH_HAS_SYSCALL_MATCH_SYM_NAME
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index 5cebb8d5a1d6..56511856ff9d 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -25,7 +25,10 @@ obj-y		:= elf.o entry-common.o irq.o opcodes.o \
>  KASAN_SANITIZE_stacktrace.o := n
>  KASAN_SANITIZE_traps.o := n
>  
> -obj-y				+= return_address.o
> +ifneq ($(CONFIG_ARM_UNWIND),y)
> +obj-$(CONFIG_FRAME_POINTER)	+= return_address.o
> +endif
> +
>  obj-$(CONFIG_ATAGS)		+= atags_parse.o
>  obj-$(CONFIG_ATAGS_PROC)	+= atags_proc.o
>  obj-$(CONFIG_DEPRECATED_PARAM_STRUCT) += atags_compat.o
> -- 
> 2.30.2
> 

This patch remove the RCU warning and the boot is now clean.

Reported-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Tested-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[Corentin Labbe]

Le Wed, Mar 02, 2022 at 12:32:01PM +0100, Ard Biesheuvel a écrit :
> Commit 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI
> unwinder") removed the dummy version of return_address() that was
> provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
> removal of the kernel_text_address() call from unwind_frame() in the
> preceding patch made it safe to do so.
> 
> However, this turns out not to be the case: Corentin reports warnings
> about suspicious RCU usage and other strange behavior that seems to
> originate in the stack unwinding that occurs in return_address().
> 
> Given that the function graph tracer (which is what these changes were
> enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
> this distinction, let's revert return_address() to the old state.
> 
> Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
> Fixes: 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Hello

On next-20220304 even with both 
ARM: unwind: avoid spurious warnings on bogus code addresses
and
ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

I got:
[    0.134043] =============================
[    0.138372] WARNING: suspicious RCU usage
[    0.142702] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
[    0.148523] -----------------------------
[    0.152850] include/linux/cgroup.h:481 suspicious rcu_dereference_check() usage!
[    0.160838] 
[    0.160838] other info that might help us debug this:
[    0.160838] 
[    0.169466] 
[    0.169466] rcu_scheduler_active = 1, debug_locks = 1
[    0.176510] 2 locks held by kthreadd/2:
[    0.180657]  #0: c19216b4 (&p->pi_lock){....}-{2:2}, at: task_rq_lock+0x34/0x134
[    0.188734]  #1: eef68b50 (&rq->__lock){-...}-{2:2}, at: task_rq_lock+0x5c/0x134
[    0.196770] 
[    0.196770] stack backtrace:
[    0.201472] CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.17.0-rc6-next-20220304-dirty #3
[    0.210018] Hardware name: Allwinner A83t board
[    0.214914]  unwind_backtrace from show_stack+0x10/0x14
[    0.220582]  show_stack from 0xf0835e8c
[    0.224818] 
[    0.226447] =============================
[    0.230774] WARNING: suspicious RCU usage
[    0.235108] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
[    0.240926] -----------------------------
[    0.245250] include/linux/cgroup.h:481 suspicious rcu_dereference_check() usage!
[    0.253235] 
[    0.253235] other info that might help us debug this:
[    0.253235] 
[    0.261868] 
[    0.261868] rcu_scheduler_active = 1, debug_locks = 1
[    0.268910] no locks held by kthreadd/2.
[    0.273132] 
[    0.273132] stack backtrace:
[    0.277822] CPU: 0 PID: 2 Comm: kthreadd Not tainted 5.17.0-rc6-next-20220304-dirty #3
[    0.286375] Hardware name: Allwinner A83t board
[    0.291265]  unwind_backtrace from show_stack+0x10/0x14
[    0.296907]  show_stack from 0xf0801efc
[    0.301220] /cpus/cpu@0 missing clock-frequency property
[    0.307034] /cpus/cpu@1 missing clock-frequency property
[    0.312879] /cpus/cpu@2 missing clock-frequency property
[    0.318683] /cpus/cpu@3 missing clock-frequency property
[    0.324531] /cpus/cpu@100 missing clock-frequency property
[    0.330533] /cpus/cpu@101 missing clock-frequency property
[    0.336584] /cpus/cpu@102 missing clock-frequency property
[    0.342600] /cpus/cpu@103 missing clock-frequency property
[    0.348581] CPU0: thread -1, cpu 0, socket 0, mpidr 80000000
[    0.357918] Setting up static identity map for 0x40100000 - 0x40100060
[    0.365942] ARM CCI driver probed
[    0.370397] sunxi multi cluster SMP support installed
[    0.377164] rcu: Hierarchical SRCU implementation.
[    0.383475] 
[    0.385118] =============================
[    0.389446] WARNING: suspicious RCU usage
[    0.393774] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
[    0.399613] -----------------------------
[    0.403948] include/linux/cgroup.h:481 suspicious rcu_dereference_check() usage!
[    0.411935] 
[    0.411935] other info that might help us debug this:
[    0.411935] 
[    0.420562] 
[    0.420562] rcu_scheduler_active = 1, debug_locks = 1
[    0.427606] 1 lock held by migration/0/12:
[    0.432030]  #0: eef68b50 (&rq->__lock){-...}-{2:2}, at: __schedule+0xf4/0xaa4
[    0.439915] 
[    0.439915] stack backtrace:
[    0.444617] CPU: 0 PID: 12 Comm: migration/0 Not tainted 5.17.0-rc6-next-20220304-dirty #3
[    0.453546] Hardware name: Allwinner A83t board
[    0.458438] Stopper: 0x0 <- 0x0
[    0.461838]  unwind_backtrace from show_stack+0x10/0x14
[    0.467488]  show_stack from 0xf087debc
[    0.473261] smp: Bringing up secondary CPUs ...
[    0.481416] CPU1: thread -1, cpu 1, socket 0, mpidr 80000001
[    0.485248] CPU2: thread -1, cpu 2, socket 0, mpidr 80000002
[    0.488578] CPU3: thread -1, cpu 3, socket 0, mpidr 80000003
[    0.492182] CPU4: thread -1, cpu 0, socket 1, mpidr 80000100
[    0.495727] CPU5: thread -1, cpu 1, socket 1, mpidr 80000101
[    0.499134] CPU6: thread -1, cpu 2, socket 1, mpidr 80000102
[    0.502435] CPU7: thread -1, cpu 3, socket 1, mpidr 80000103
[    0.502967] smp: Brought up 1 node, 8 CPUs
[    0.550416] SMP: Total of 8 processors activated (384.00 BogoMIPS).
[    0.557181] CPU: All CPU(s) started in SVC mode.
[    0.565790] devtmpfs: initialized
[    0.606240] VFP support v0.3: implementor 41 architecture 2 part 30 variant 7 rev 5
[    0.643705] DMA-API: preallocated 65536 debug entries
[    0.649376] DMA-API: debugging enabled by kernel config
[    0.655034] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[    0.665830] futex hash table entries: 2048 (order: 5, 131072 bytes, linear)
[    0.675593] pinctrl core: initialized pinctrl subsystem
[    0.686800] NET: Registered PF_NETLINK/PF_ROUTE protocol family
[    0.699502] DMA: preallocated 256 KiB pool for atomic coherent allocations
[    0.712694] thermal_sys: Registered thermal governor 'step_wise'
[    0.715954] hw-breakpoint: found 5 (+1 reserved) breakpoint and 4 watchpoint registers.
[    0.731191] hw-breakpoint: maximum watchpoint size is 8 bytes.
[    0.766642] platform 1c0c000.lcd-controller: Fixing up cyclic dependency with 1200000.mixer
[    0.776145] platform 1c0c000.lcd-controller: Fixing up cyclic dependency with 1100000.mixer
[    0.787258] platform 1c0d000.lcd-controller: Fixing up cyclic dependency with 1200000.mixer
[    0.796657] platform 1c0d000.lcd-controller: Fixing up cyclic dependency with 1100000.mixer
[    0.831104] platform 1ee0000.hdmi: Fixing up cyclic dependency with 1c0d000.lcd-controller
[    0.851849] platform connector: Fixing up cyclic dependency with 1ee0000.hdmi
[    0.903687] SCSI subsystem initialized
[    0.909645] pps_core: LinuxPPS API ver. 1 registered
[    0.915166] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <giometti@linux.it>
[    0.925126] PTP clock support registered
[    0.934551] 
[    0.936221] =============================
[    0.940560] WARNING: suspicious RCU usage
[    0.944898] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
[    0.950826] -----------------------------
[    0.955232] include/linux/cgroup.h:481 suspicious rcu_dereference_check() usage!
[    0.963184] 
[    0.963184] other info that might help us debug this:
[    0.963184] 
[    0.971976] 
[    0.971976] rcu_scheduler_active = 2, debug_locks = 1
[    0.979142] 1 lock held by watchdogd/58:
[    0.983377]  #0: eefd8b50 (&rq->__lock){-.-.}-{2:2}, at: __schedule+0xf4/0xaa4
[    0.991367] 
[    0.991367] stack backtrace:
[    0.996051] CPU: 7 PID: 58 Comm: watchdogd Not tainted 5.17.0-rc6-next-20220304-dirty #3
[    1.004928] Hardware name: Allwinner A83t board
[    1.009789]  unwind_backtrace from show_stack+0x10/0x14
[    1.015443]  show_stack from 0xf0991e6c

On next-20220303 I got nothing.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[Ard Biesheuvel]

On Sat, 5 Mar 2022 at 20:20, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
>
> Le Wed, Mar 02, 2022 at 12:32:01PM +0100, Ard Biesheuvel a écrit :
> > Commit 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI
> > unwinder") removed the dummy version of return_address() that was
> > provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
> > removal of the kernel_text_address() call from unwind_frame() in the
> > preceding patch made it safe to do so.
> >
> > However, this turns out not to be the case: Corentin reports warnings
> > about suspicious RCU usage and other strange behavior that seems to
> > originate in the stack unwinding that occurs in return_address().
> >
> > Given that the function graph tracer (which is what these changes were
> > enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
> > this distinction, let's revert return_address() to the old state.
> >
> > Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
> > Fixes: 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Hello
>
> On next-20220304 even with both
> ARM: unwind: avoid spurious warnings on bogus code addresses
> and
> ARM: return_address: disable again for CONFIG_ARM_UNWIND=y
>
> I got:
> [    0.134043] =============================
> [    0.138372] WARNING: suspicious RCU usage
> [    0.142702] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
...
> [    1.015443]  show_stack from 0xf0991e6c
>
> On next-20220303 I got nothing.

This makes me suspect that this is unrelated, given that no changes
were applied to Russell's tree in the mean time, as far as I can tell.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

[Corentin Labbe]

Le Sat, Mar 05, 2022 at 10:04:23PM +0000, Ard Biesheuvel a écrit :
> On Sat, 5 Mar 2022 at 20:20, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
> >
> > Le Wed, Mar 02, 2022 at 12:32:01PM +0100, Ard Biesheuvel a écrit :
> > > Commit 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI
> > > unwinder") removed the dummy version of return_address() that was
> > > provided for the CONFIG_ARM_UNWIND=y case, on the assumption that the
> > > removal of the kernel_text_address() call from unwind_frame() in the
> > > preceding patch made it safe to do so.
> > >
> > > However, this turns out not to be the case: Corentin reports warnings
> > > about suspicious RCU usage and other strange behavior that seems to
> > > originate in the stack unwinding that occurs in return_address().
> > >
> > > Given that the function graph tracer (which is what these changes were
> > > enabling for CONFIG_ARM_UNWIND=y builds) does not appear to care about
> > > this distinction, let's revert return_address() to the old state.
> > >
> > > Cc: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > Fixes: 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > Hello
> >
> > On next-20220304 even with both
> > ARM: unwind: avoid spurious warnings on bogus code addresses
> > and
> > ARM: return_address: disable again for CONFIG_ARM_UNWIND=y
> >
> > I got:
> > [    0.134043] =============================
> > [    0.138372] WARNING: suspicious RCU usage
> > [    0.142702] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
> ...
> > [    1.015443]  show_stack from 0xf0991e6c
> >
> > On next-20220303 I got nothing.
> 
> This makes me suspect that this is unrelated, given that no changes
> were applied to Russell's tree in the mean time, as far as I can tell.

Hello

I bisected to dc6e0818bc9a0336d9accf3ea35d146d72aa7a18 ("sched/cpuacct: Optimize away RCU read lock")
but reverting lead to some remaining RCU (but far less than before).

I continue to debug it.

Regards

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel