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

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

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;
}