Re: [PATCH] seqlock: Use WRITE_ONCE() when updating sequence

From: Waiman Long
Date: Wed Dec 18 2024 - 13:38:32 EST


On 12/18/24 11:57 AM, Paul E. McKenney wrote:
On Wed, Dec 18, 2024 at 11:10:01AM -0500, Waiman Long wrote:
On 12/18/24 10:45 AM, Paul E. McKenney wrote:
On Tue, Dec 17, 2024 at 10:30:39PM -0500, Waiman Long wrote:
For seqcount, its actual value isn't important. What is important is whether
the value changes and whether it is even or odd. So even if store tearing is
happening, it shouldn't affect its operation. I doubt we need to use
WRITE_ONCE() here. Could you come up with a scenario where store tearing
will make it behave incorrectly?
But why expand the state space?
I don't quite get what you mean by "state space".
The state space includes the set of possible values that might be stored
in .sequence at any given point in time. So if you allow the compiler to
tear the store, you are increasing the number of values that a concurrent
reader can return, and thus also increasing the number of states that must
be considered. On the other hand, if you use WRITE_ONCE() to constrain
the compiler to do a single store, then you don't have to even worry
about the added states due to these partial updates.

Thanks for the explanation. Now I know what you mean.

As I said above, the correct operation of seqcount does not depend on getting its intended value right. It is just its odd/even-ness as well as if the value has been changed from previous read. That is the state space where seqcount is operated on.

Also, there are potentially "optimizations" other than store tearing.
No, I haven't seen them yet, but then again, there were a great many
optimizations that were not being used back when I started coding C.
All the inc's are bounded by smp_wmb()'s which should limit the kind of
optimizations that can be done by the compiler. That is why I don't see a
need to use WRITE_ONCE() here. Of course, there may be some corner cases
that I may miss. So I ask for whether such a case exists.
One other potential optimization is using the variable for scratch storage
immediately prior to the update, which could store any value there.
I haven't seen this myself, but the standard permits it, and in some
cases register pressure might encourage it.

And who knows what else our creative compiler writers will come up with?

If the compiler really uses the variable as a scratch storage, it will be a problem if the variable can be accessed concurrently from multiple CPUs. It is a new compiler optimization strategy that I am aware before. In that case, we may really need a way to mark these variables as not suitable for such advanced optimization.

Thanks,
Longman


Thanx, Paul