Skip to contentSkip to conversation

ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

PATCH In review Ard Biesheuvel 2 participants 5 messages Mar 2, 2022 – Mar 7, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: return_address: disable again for CONFIG_ARM_UNWIND=y

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 <[email protected]>
Fixes 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
---
 arch/arm/include/asm/ftrace.h | 18 ++++++++++++++++++
 arch/arm/kernel/Makefile      |  5 ++++-
 2 files changed, 22 insertions(+), 1 deletion(-)
arch/arm/include/asm/ftrace.h +18 -0
index 5358aad67831..7e9251ca29fe 100644
@@ -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
arch/arm/kernel/Makefile +4 -1
index 5cebb8d5a1d6..56511856ff9d 100644
@@ -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
Signature · 7 lines
2.30.2


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Tested-by on v1 1/1
#1 lore raw
Quoted 72 lines · 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 <[email protected]>
Fixes 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")
---
 arch/arm/include/asm/ftrace.h | 18 ++++++++++++++++++
 arch/arm/kernel/Makefile      |  5 ++++-
 2 files changed, 22 insertions(+), 1 deletion(-)
arch/arm/include/asm/ftrace.h +18 -0
index 5358aad67831..7e9251ca29fe 100644
@@ -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
arch/arm/kernel/Makefile +4 -1
index 5cebb8d5a1d6..56511856ff9d 100644
@@ -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
Signature · 1 line
2.30.2

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

Thanks!

#2 lore raw
In reply to Ard Biesheuvel
Quoted 10 lines · 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().

Quoted 7 lines

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 <[email protected]>
Fixes 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")

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 <[email protected]>
[    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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#3 lore raw
Quoted 31 lines · Corentin Labbe
Quoted 10 lines · 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().

Quoted 7 lines

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 <[email protected]>
Fixes 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")

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

...

Quoted 3 lines
[    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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

#4 lore raw
Quoted 32 lines · Ard Biesheuvel
Quoted 27 lines · Corentin Labbe
Quoted 10 lines · 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().

Quoted 7 lines

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 <[email protected]>
Fixes 41918ec82eb6 ("ARM: ftrace: enable the graph tracer with the EABI unwinder")

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

Quoted 4 lines

I got:

[    0.134043] =============================
[    0.138372] WARNING: suspicious RCU usage
[    0.142702] 5.17.0-rc6-next-20220304-dirty #3 Not tainted
Quoted 7 lines

...

Quoted 3 lines
[    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
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel