linux/arch/arm64/include/asm/rwonce.h

83 lines
2.3 KiB

arm64: Optimize __READ_ONCE() with CONFIG_LTO=y Rework arm64 LTO __READ_ONCE() to improve code generation as follows: 1. Replace _Generic-based __unqual_scalar_typeof() with more complete __rwonce_typeof_unqual(). This strips qualifiers from all types, not just integer types, which is required to be able to assign (must be non-const) to __u.__val in the non-atomic case (required for #2). One subtle point here is that non-integer types of __val could be const or volatile within the union with the old __unqual_scalar_typeof(), if the passed variable is const or volatile. This would then result in a forced load from the stack if __u.__val is volatile; in the case of const, it does look odd if the underlying storage changes, but the compiler is told said member is "const" -- it smells like UB. 2. Eliminate the atomic flag and ternary conditional expression. Move the fallback volatile load into the default case of the switch, ensuring __u is unconditionally initialized across all paths. The statement expression now unconditionally returns __u.__val. This refactoring appears to help the compiler improve (or fix) code generation. With a defconfig + LTO + debug options builds, we observe different codegen for the following functions: btrfs_reclaim_sweep (708 -> 1032 bytes) btrfs_sinfo_bg_reclaim_threshold_store (200 -> 204 bytes) check_mem_access (3652 -> 3692 bytes) [inlined bpf_map_is_rdonly] console_flush_all (1268 -> 1264 bytes) console_lock_spinning_disable_and_check (180 -> 176 bytes) igb_add_filter (640 -> 636 bytes) igb_config_tx_modes (2404 -> 2400 bytes) kvm_vcpu_on_spin (480 -> 476 bytes) map_freeze (376 -> 380 bytes) netlink_bind (1664 -> 1656 bytes) nmi_cpu_backtrace (404 -> 400 bytes) set_rps_cpu (516 -> 520 bytes) swap_cluster_readahead (944 -> 932 bytes) tcp_accecn_third_ack (328 -> 336 bytes) tcp_create_openreq_child (1764 -> 1772 bytes) tcp_data_queue (5784 -> 5892 bytes) tcp_ecn_rcv_synack (620 -> 628 bytes) xen_manage_runstate_time (944 -> 896 bytes) xen_steal_clock (340 -> 296 bytes) Increase of some functions are due to more aggressive inlining due to better codegen (in this build, e.g. bpf_map_is_rdonly is no longer present due to being inlined completely). NOTE: The return-value-of-function-drops-qualifiers hack was first suggested by Al Viro in [1], which notes some of its limitations which make it unsuitable for a general __unqual_scalar_typeof() replacement. Notably, array types are not supported, and GCC 8.1-8.3 still fail. Why should we use it here? READ_ONCE() does not support reading whole arrays, and the GCC version problem only affects 3 minor releases of a very ancient still-supported GCC version; not only that, this arm64 READ_ONCE() version is currently only activated by LTO builds, which to-date are *only supported by Clang*! Link: https://lore.kernel.org/all/20260111182010.GH3634291@ZenIV/ [1] Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Will Deacon <will@kernel.org>
2026-02-16 15:16:22 +01:00
arm64, compiler-context-analysis: Permit alias analysis through __READ_ONCE() with CONFIG_LTO=y When enabling Clang's Context Analysis (aka. Thread Safety Analysis) on kernel/futex/core.o (see Peter's changes at [1]), in arm64 LTO builds we could see: | kernel/futex/core.c:982:1: warning: spinlock 'atomic ? __u.__val : q->lock_ptr' is still held at the end of function [-Wthread-safety-analysis] | 982 | } | | ^ | kernel/futex/core.c:976:2: note: spinlock acquired here | 976 | spin_lock(lock_ptr); | | ^ | kernel/futex/core.c:982:1: warning: expecting spinlock 'q->lock_ptr' to be held at the end of function [-Wthread-safety-analysis] | 982 | } | | ^ | kernel/futex/core.c:966:6: note: spinlock acquired here | 966 | void futex_q_lockptr_lock(struct futex_q *q) | | ^ | 2 warnings generated. Where we have: extern void futex_q_lockptr_lock(struct futex_q *q) __acquires(q->lock_ptr); .. void futex_q_lockptr_lock(struct futex_q *q) { spinlock_t *lock_ptr; /* * See futex_unqueue() why lock_ptr can change. */ guard(rcu)(); retry: >> lock_ptr = READ_ONCE(q->lock_ptr); spin_lock(lock_ptr); ... } At the time of the above report (prior to removal of the 'atomic' flag), Clang Thread Safety Analysis's alias analysis resolved 'lock_ptr' to 'atomic ? __u.__val : q->lock_ptr' (now just '__u.__val'), and used this as the identity of the context lock given it cannot "see through" the inline assembly; however, we want 'q->lock_ptr' as the canonical context lock. While for code generation the compiler simplified to '__u.__val' for pointers (8 byte case -> 'atomic' was set), TSA's analysis (a) happens much earlier on the AST, and (b) would be the wrong deduction. Now that we've gotten rid of the 'atomic' ternary comparison, we can return '__u.__val' through a pointer that we initialize with '&x', but then update via a pointer-to-pointer. When READ_ONCE()'ing a context lock pointer, TSA's alias analysis does not invalidate the initial alias when updated through the pointer-to-pointer, and we make it effectively "see through" the __READ_ONCE(). Code generation is unchanged. Link: https://lkml.kernel.org/r/20260121110704.221498346@infradead.org [1] Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202601221040.TeM0ihff-lkp@intel.com/ Cc: Peter Zijlstra <peterz@infradead.org> Tested-by: Boqun Feng <boqun@kernel.org> Reviewed-by: David Laight <david.laight.linux@gmail.com> Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Will Deacon <will@kernel.org>
2026-02-16 15:16:23 +01:00
arm64: Optimize __READ_ONCE() with CONFIG_LTO=y Rework arm64 LTO __READ_ONCE() to improve code generation as follows: 1. Replace _Generic-based __unqual_scalar_typeof() with more complete __rwonce_typeof_unqual(). This strips qualifiers from all types, not just integer types, which is required to be able to assign (must be non-const) to __u.__val in the non-atomic case (required for #2). One subtle point here is that non-integer types of __val could be const or volatile within the union with the old __unqual_scalar_typeof(), if the passed variable is const or volatile. This would then result in a forced load from the stack if __u.__val is volatile; in the case of const, it does look odd if the underlying storage changes, but the compiler is told said member is "const" -- it smells like UB. 2. Eliminate the atomic flag and ternary conditional expression. Move the fallback volatile load into the default case of the switch, ensuring __u is unconditionally initialized across all paths. The statement expression now unconditionally returns __u.__val. This refactoring appears to help the compiler improve (or fix) code generation. With a defconfig + LTO + debug options builds, we observe different codegen for the following functions: btrfs_reclaim_sweep (708 -> 1032 bytes) btrfs_sinfo_bg_reclaim_threshold_store (200 -> 204 bytes) check_mem_access (3652 -> 3692 bytes) [inlined bpf_map_is_rdonly] console_flush_all (1268 -> 1264 bytes) console_lock_spinning_disable_and_check (180 -> 176 bytes) igb_add_filter (640 -> 636 bytes) igb_config_tx_modes (2404 -> 2400 bytes) kvm_vcpu_on_spin (480 -> 476 bytes) map_freeze (376 -> 380 bytes) netlink_bind (1664 -> 1656 bytes) nmi_cpu_backtrace (404 -> 400 bytes) set_rps_cpu (516 -> 520 bytes) swap_cluster_readahead (944 -> 932 bytes) tcp_accecn_third_ack (328 -> 336 bytes) tcp_create_openreq_child (1764 -> 1772 bytes) tcp_data_queue (5784 -> 5892 bytes) tcp_ecn_rcv_synack (620 -> 628 bytes) xen_manage_runstate_time (944 -> 896 bytes) xen_steal_clock (340 -> 296 bytes) Increase of some functions are due to more aggressive inlining due to better codegen (in this build, e.g. bpf_map_is_rdonly is no longer present due to being inlined completely). NOTE: The return-value-of-function-drops-qualifiers hack was first suggested by Al Viro in [1], which notes some of its limitations which make it unsuitable for a general __unqual_scalar_typeof() replacement. Notably, array types are not supported, and GCC 8.1-8.3 still fail. Why should we use it here? READ_ONCE() does not support reading whole arrays, and the GCC version problem only affects 3 minor releases of a very ancient still-supported GCC version; not only that, this arm64 READ_ONCE() version is currently only activated by LTO builds, which to-date are *only supported by Clang*! Link: https://lore.kernel.org/all/20260111182010.GH3634291@ZenIV/ [1] Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Will Deacon <will@kernel.org>
2026-02-16 15:16:22 +01:00
arm64, compiler-context-analysis: Permit alias analysis through __READ_ONCE() with CONFIG_LTO=y When enabling Clang's Context Analysis (aka. Thread Safety Analysis) on kernel/futex/core.o (see Peter's changes at [1]), in arm64 LTO builds we could see: | kernel/futex/core.c:982:1: warning: spinlock 'atomic ? __u.__val : q->lock_ptr' is still held at the end of function [-Wthread-safety-analysis] | 982 | } | | ^ | kernel/futex/core.c:976:2: note: spinlock acquired here | 976 | spin_lock(lock_ptr); | | ^ | kernel/futex/core.c:982:1: warning: expecting spinlock 'q->lock_ptr' to be held at the end of function [-Wthread-safety-analysis] | 982 | } | | ^ | kernel/futex/core.c:966:6: note: spinlock acquired here | 966 | void futex_q_lockptr_lock(struct futex_q *q) | | ^ | 2 warnings generated. Where we have: extern void futex_q_lockptr_lock(struct futex_q *q) __acquires(q->lock_ptr); .. void futex_q_lockptr_lock(struct futex_q *q) { spinlock_t *lock_ptr; /* * See futex_unqueue() why lock_ptr can change. */ guard(rcu)(); retry: >> lock_ptr = READ_ONCE(q->lock_ptr); spin_lock(lock_ptr); ... } At the time of the above report (prior to removal of the 'atomic' flag), Clang Thread Safety Analysis's alias analysis resolved 'lock_ptr' to 'atomic ? __u.__val : q->lock_ptr' (now just '__u.__val'), and used this as the identity of the context lock given it cannot "see through" the inline assembly; however, we want 'q->lock_ptr' as the canonical context lock. While for code generation the compiler simplified to '__u.__val' for pointers (8 byte case -> 'atomic' was set), TSA's analysis (a) happens much earlier on the AST, and (b) would be the wrong deduction. Now that we've gotten rid of the 'atomic' ternary comparison, we can return '__u.__val' through a pointer that we initialize with '&x', but then update via a pointer-to-pointer. When READ_ONCE()'ing a context lock pointer, TSA's alias analysis does not invalidate the initial alias when updated through the pointer-to-pointer, and we make it effectively "see through" the __READ_ONCE(). Code generation is unchanged. Link: https://lkml.kernel.org/r/20260121110704.221498346@infradead.org [1] Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202601221040.TeM0ihff-lkp@intel.com/ Cc: Peter Zijlstra <peterz@infradead.org> Tested-by: Boqun Feng <boqun@kernel.org> Reviewed-by: David Laight <david.laight.linux@gmail.com> Signed-off-by: Marco Elver <elver@google.com> Signed-off-by: Will Deacon <will@kernel.org>
2026-02-16 15:16:23 +01:00