Quoting Daniel Vetter (2019-08-14 18:06:26)
On Wed, Aug 14, 2019 at 05:42:48PM +0100, Chris Wilson wrote:
Quoting Daniel Vetter (2019-08-14 16:39:08)
[snip]
if (old)
old->shared_count = 0;write_seqcount_end(&obj->seq);
smp_store_mb(old->shared_count, 0);So your comment and the kerneldoc don't match up. Quoting Documentation/memory-barriers.txt:
This assigns the value to the variable and then inserts a full memory barrier after it. It isn't guaranteed to insert anything more than a compiler barrier in a UP compilation.So order is 1. store 2. fence, but your comment suggests you want it the other way round.
What's more weird is that it is a fully serialising instruction that is used to fence first as part of the update. If that's way PeterZ uses it...
I haven't looked at the implementations tbh, just going with the text. Or do you mean in the write_seqlock that we're replacing?
Nah, I misremembered set_current_state(), all that implies is the fence is before the following instructions. I have some recollection that it can be used as a RELEASE operation (if only because it is a locked xchg). If all else fails, make it an xchg_release(). Or normal assignment + smp_wmb().
It's an exclusive fence. If it is replaced, it must be later than all the shared fences (and dependent on them directly or indirectly), and so still a consistent snapshot.
I'm not worried about the fence, that part is fine. But we're defacto using the fence as a fancy seqlock-of-sorts. And if the fence gets reused and the pointers match, then our seqlock-of-sorts breaks. But I haven't looked around whether there's more in the code that makes this an irrelevant issue.
No, it should not break if we replace the fence with the same pointer. If the fence pointer expires, reused and assigned back as the excl_fence -- it is still the excl_fence and by the properties of that excl_fence construction, it is later than the shared_fences. -Chris