Messages in this thread | | | Date | Tue, 10 Jan 2023 11:27:28 +0000 | From | Mark Rutland <> | Subject | Re: [RFC][PATCH 08/12] s390: Replace cmpxchg_double() with cmpxchg128() |
| |
On Tue, Jan 10, 2023 at 09:32:55AM +0100, Peter Zijlstra wrote: > On Tue, Jan 10, 2023 at 08:23:05AM +0100, Heiko Carstens wrote: > > > So, Alexander Gordeev reported that this code was already prior to your > > changes potentially broken with respect to missing READ_ONCE() within the > > cmpxchg_double() loops. > > Unless there's an early exit, that shouldn't matter. If you managed to > read garbage the cmpxchg itself will simply fail and the loop retries.
I don't think that's true; without READ_ONCE() the compiler could (but is very unlikely to) read multiple times, and that could cause problems.
For example:
| prev = *ptr; | | do { | new = some_function_of(prev); | old = cmpxchg(ptr, prev, new); | } while (old != prev);
Could effectively become:
| prev1 = *ptr; | prev2 = *ptr; | | do { | new = some_function_of(prev1) | old = cmpxchg(ptr, prev2, new); | } while (old != prev2);
... which would effectively udpate from a stale value, throwing away prev2. That and the two generated reads could be in either order.
So I do think it's warranted to use READ_ONCE() for the prev value feeding into a cmpxchg operation, even if that's only for the "once" part rather than lack of tearing.
Thanks, Mark.
| |