Re: [PATCH] elf: Fix fences in _dl_find_object_update (bug 28745)

[Florian Weimer <fweimer@redhat.com>]

* Szabolcs Nagy:

> The 01/05/2022 14:47, Florian Weimer wrote:
>> As explained in Hans Boehm, Can Seqlocks Get Along with Programming
>> Language Memory Models?, an acquire fence is needed in
>> _dlfo_read_success.  The lack of a fence was
>> 
>> The fence in _dlfo_mappings_begin_update has been reordered, turning
>> the fence/store sequence into a release MO store equivalent.
>> 
>
> now i don't fully understand why we need the +2 then +1 trick.
>
> the writer is like
>
>   v = load (&ver);
>   i = v & 1;
>   fence ();
>   fetch_add (&ver, 2);
>   update (!i);
>   fence ();
>   fetch_add (&ver, 1);
>
> why not
>
>   v = load (&ver);
>   i = v & 1;
>   fence ();
>   update (!i);
>   fence ();
>   store (&ver, v+1);
>
> i.e. i'd expect readers to only need to detect an interleaving
> "commit" operation (final store to ver). for which we need
>
> 1) updates are not visible too early (before previous commit)
> 2) updates are visible after commit.
>
> and i think two release fences can take care of this (even
> with relaxed store).
>
> i think on cppmem 1) can be modelled as
>
> int main() {
>   atomic_int v=0;
>   atomic_int x=0;
>   {{{ {
>    v.store(1,mo_relaxed); // prev commit
>    atomic_thread_fence(mo_release);
>    x.store(1,mo_relaxed);
>   } ||| {
>    v.load(mo_acquire).readsvalue(0);
>    x.load(mo_relaxed).readsvalue(1);
>    atomic_thread_fence(mo_acquire);
>    v.load(mo_relaxed).readsvalue(0);
>   } }}}
>   return 0;
> }
>
> while 2) can be modelled as
>
> int main() {
>   atomic_int v=0;
>   atomic_int x=0;
>   {{{ {
>    x.store(1,mo_relaxed);
>    atomic_thread_fence(mo_release);
>    v.store(1,mo_relaxed);
>   } ||| {
>    v.load(mo_acquire).readsvalue(1);
>    x.load(mo_relaxed).readsvalue(0);
>    atomic_thread_fence(mo_acquire);
>    v.load(mo_relaxed).readsvalue(1);
>   } }}}
>   return 0;
> }

I think you are right.

I want to make this change in a separate commit.  The present patch
already lumps together unrelated changes (the new fence, the reordering
of one of the existing fences, and the relaxed MO loads/stores for the
TM data).

Thanks,
Florian