Re: [PATCH hazptr v2 2/4] ptreq.h: Introduce ptr_eq() to preserve address dependency

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

On 2026-10-09 17:55, Gary Guo wrote:
> On Fri Oct 9, 2026 at 7:21 PM BST, Mathieu Desnoyers wrote:
>> Compiler CSE and SSA GVN optimizations can cause the address dependency
>> of addresses returned by rcu_dereference to be lost when comparing those
>> pointers with either constants or previously loaded pointers.
>>
>> Introduce ptr_eq() to compare two addresses while preserving the address
>> dependencies for later use of the address. It should be used when
>> comparing an address returned by rcu_dereference().
>>
>> This is needed to prevent the compiler CSE and SSA GVN optimizations
>> from using @a (or @b) in places where the source refers to @b (or @a)
>> based on the fact that after the comparison, the two are known to be
>> equal, which does not preserve address dependencies and allows the
>> following misordering speculations:
>>
>> - If @b is a constant, the compiler can issue the loads which depend
>>    on @a before loading @a.
>> - If @b is a register populated by a prior load, weakly-ordered
>>    CPUs can speculate loads which depend on @a before loading @a.
>>
>> The same logic applies with @a and @b swapped.
>>
>> The header "linux/ptreq.h" is the expected include target. It
>> contains the documentation of the ptr_eq() API.
>>
>> An architecture specific implementation of the pointer comparison
>> can be implemented by each architecture as asm/ptreq.h. The x86
>> implementation is provided initially. If no implementation header is
>> present for the architecture, an arch-agnostic fallback based on
>> OPTIMIZER_HIDE_VAR is included from asm-generic.
>>
>> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>> Suggested-by: Boqun Feng <boqun.feng@gmail.com>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> Cc: "Paul E. McKenney" <paulmck@kernel.org>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Boqun Feng <boqun.feng@gmail.com>
>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> Cc: John Stultz <jstultz@google.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Cc: Boqun Feng <boqun.feng@gmail.com>
>> Cc: Frederic Weisbecker <frederic@kernel.org>
>> Cc: Joel Fernandes <joel@joelfernandes.org>
>> Cc: Josh Triplett <josh@joshtriplett.org>
>> Cc: Uladzislau Rezki <urezki@gmail.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Lai Jiangshan <jiangshanlai@gmail.com>
>> Cc: Zqiang <qiang.zhang1211@gmail.com>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Waiman Long <longman@redhat.com>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: maged.michael@gmail.com
>> Cc: Mateusz Guzik <mjguzik@gmail.com>
>> Cc: Gary Guo <gary@garyguo.net>
>> Cc: rcu@vger.kernel.org
>> Cc: linux-mm@kvack.org
>> Cc: lkmm@lists.linux.dev
>> Cc: Nikita Popov <github@npopov.com>
>> Cc: llvm@lists.linux.dev
>> Cc: Lian Wang <lianux.mm@gmail.com>
>> Cc: Kunwu Chan <kunwu.chan@gmail.com>
>> ---
>> Changes since v1:
>> - Move to linux/ptreq.h, implement x86-specific comparison
>>    and asm-generic header (fallback).
>>
>> Changes since v0:
>> - Include feedback from Alan Stern.
>> ---
>>   arch/x86/include/asm/ptreq.h | 16 +++++++++
>>   include/asm-generic/Kbuild   |  1 +
>>   include/asm-generic/ptreq.h  | 16 +++++++++
> 
> I agree with Boqun that this should be ptr_eq. There is no need to save that
> character :)
> 
>>   include/linux/ptreq.h        | 63 ++++++++++++++++++++++++++++++++++++
>>   4 files changed, 96 insertions(+)
>>   create mode 100644 arch/x86/include/asm/ptreq.h
>>   create mode 100644 include/asm-generic/ptreq.h
>>   create mode 100644 include/linux/ptreq.h
>>
>> diff --git a/arch/x86/include/asm/ptreq.h b/arch/x86/include/asm/ptreq.h
>> new file mode 100644
>> index 000000000000..593b45f80451
>> --- /dev/null
>> +++ b/arch/x86/include/asm/ptreq.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_X86_PTREQ_H
>> +#define _ASM_X86_PTREQ_H
>> +
>> +#include <linux/types.h>
>> +#include <asm/asm.h>
>> +
>> +static __always_inline
>> +bool ptr_eq(const volatile void *a, const volatile void *b)
>> +{
>> +	bool ret;
>> +	asm(__ASM_SIZE(cmp) " %1,%2" : "=@ccz" (ret) : "r" (a), "r" (b));
>> +	return ret;
>> +}
>> +
>> +#endif	/* _ASM_X86_PTREQ_H */
>> diff --git a/include/asm-generic/Kbuild b/include/asm-generic/Kbuild
>> index 2bc00c67dc54..e1d95dea09d5 100644
>> --- a/include/asm-generic/Kbuild
>> +++ b/include/asm-generic/Kbuild
>> @@ -47,6 +47,7 @@ mandatory-y += percpu.h
>>   mandatory-y += percpu_types.h
>>   mandatory-y += pgalloc.h
>>   mandatory-y += preempt.h
>> +mandatory-y += ptreq.h
>>   mandatory-y += rqspinlock.h
>>   mandatory-y += runtime-const.h
>>   mandatory-y += rwonce.h
>> diff --git a/include/asm-generic/ptreq.h b/include/asm-generic/ptreq.h
>> new file mode 100644
>> index 000000000000..db5a88628b1e
>> --- /dev/null
>> +++ b/include/asm-generic/ptreq.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_GENERIC_PTREQ_H
>> +#define __ASM_GENERIC_PTREQ_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/compiler.h>
>> +
>> +static __always_inline
>> +bool ptr_eq(const volatile void *a, const volatile void *b)
> 
> I suppose this is `const volatile` to take advantage of implicit conversion that
> adds cv-qualifiers to be added implicitly. Might worth a mention in somewhere
> (commit message maybe) because I was confused for a second on why this needs
> to be volatile.
> 
>> +{
>> +	OPTIMIZER_HIDE_VAR(a);
>> +	OPTIMIZER_HIDE_VAR(b);
>> +	return a == b;
>> +}
> 
> One thought on inline assembly. For RISC-V, branching with comparison is a
> single instruction, but there's no instruction for obtaining the boolean for
> a == b.
> 
> One option is to do
> 
>      static __always_inline
>      bool ptr_eq_likely(const volatile void *a, const volatile void *b) {
>          asm goto ("bne %0, %1, %l2" : : "r" (a), "r" (b) :: not_eq);
>          return true;
>      not_eq:
>          return likely(false);
>      }
> 
> and then the optimizer can turn a ptr_eq call + a branch to a single
> instruction. This, however, would require likely + unlikely variants.
> 
> Alternatively, we can just not have all the complexity, and just emit one extra
> instruction:
> 
>      static __always_inline
>      bool ptr_eq(const volatile void *a, const volatile void *b) {
>          unsigned long diff;
>          asm("xor %0, %1, %2" : "=r" (diff) : "r" (a), "r" (b));
>          return diff == 0;
>      }
> 
> Thoughts?

If architectures end up needing the likely/unlikely variants we can
do that if it saves instructions on the hot path. How about we do this
change when we introduce optimized support for riscv ?

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com