On 2024-09-28 16:58, Alan Stern wrote:
On Sat, Sep 28, 2024 at 09:51:28AM -0400, Mathieu Desnoyers wrote:
[...]
-- Be very careful about comparing pointers obtained from
- rcu_dereference() against non-NULL values. As Linus Torvalds
- explained, if the two pointers are equal, the compiler could
- substitute the pointer you are comparing against for the pointer
- obtained from rcu_dereference(). For example::
+- Use relational operators which preserve address dependencies
+ (such as "ptr_eq()") to compare pointers obtained from
Nit: ptr_eq() is an inline function, not a relational operator. Say
"operations that" instead of "relational operators which".
+ rcu_dereference() against non-NULL values or against pointers
Note: here I need to update the wording as well:
+- Use operations that preserve address dependencies (such as
+ "ptr_eq()") to compare pointers obtained from rcu_dereference()
+ against non-NULL pointers. As Linus Torvalds explained, if the
+ two pointers are equal, the compiler could substitute the
+ pointer you are comparing against for the pointer obtained from
+ rcu_dereference(). For example::
+ obtained from prior loads. As Linus Torvalds explained, if the
+ two pointers are equal, the compiler could substitute the
+ pointer you are comparing against for the pointer obtained from
+ rcu_dereference(). For example::
p = rcu_dereference(gp);
if (p == &default_struct)
@@ -125,6 +127,23 @@ readers working properly:
On ARM and Power hardware, the load from "default_struct.a"
can now be speculated, such that it might happen before the
rcu_dereference(). This could result in bugs due to misordering.
+ Performing the comparison with "ptr_eq()" ensures the compiler
+ does not perform such transformation.
+
+ If the comparison is against a pointer obtained from prior
+ loads, the compiler is allowed to use either register for the
This is true even when the comparison is against a pointer obtained from
a later load. Just say "another pointer" instead of "a pointer obtained
from prior loads". (And why would someone need multiple loads to
obtain a single pointer?)
Also, say "pointer" instead of "register".
OK.
+ following accesses, which loses the address dependency and
+ allows weakly-ordered architectures such as ARM and PowerPC
+ to speculate the address-dependent load before rcu_dereference().
+ For example::
+
+ p1 = READ_ONCE(gp);
+ p2 = rcu_dereference(gp);
+ if (p1 == p2)
+ do_default(p2->a);
Here you should say that the compiler could use p1->a rather than p2->a,
destroying the address dependency. That's the whole point of this; you
shouldn't skip over it.
OK.
+
+ Performing the comparison with "ptr_eq()" ensures the compiler
+ preserves the address dependencies.
However, comparisons are OK in the following cases:
@@ -204,6 +223,11 @@ readers working properly:
comparison will provide exactly the information that the
compiler needs to deduce the value of the pointer.
+ When in doubt, use relational operators that preserve address
Again, "operations" instead of "relational operators".
OK. Will fix in my next round.
Thanks,
Mathieu
Alan Stern
+ dependencies (such as "ptr_eq()") to compare pointers obtained
+ from rcu_dereference() against non-NULL values or against
+ pointers obtained from prior loads.
+
- Disable any value-speculation optimizations that your compiler
might provide, especially if you are making use of feedback-based
optimizations that take data collected from prior runs. Such
--
2.39.2
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
