Re: [PATCH] Use C11 atomics instead of atomic_bit_set/bit_test_set

[Florian Weimer <fweimer@redhat.com>]

* Wilco Dijkstra:

> Hi Florian,
>
>> Why is acquire MO required here?  I don't see any synchronizing store.
>> Isn't this mostly a compiler barrier for use-after-free detection?
>
> You're right, it looks like the only reason for atomic is to ensure memory
> is only freed once. A few cancelhandling accesses use acquire, and there
> are various relaxed loads and even non-atomic loads of it, but not a
> single release store, so there is no use for acquire MO here.
>
> Cheers,
> Wilco
>
>
> v2: Use relaxed atomics since there is no MO dependence
>
> Replace the 3 uses of atomic_bit_set and atomic_bit_test_set with
> atomic_fetch_or_relaxed.
>
> ---
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 29fa71b3b2a3d0a671149eaf619e4d518c56aef5..ecec901b14f602e3c93da1a847f043ffee41a1f4 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -2460,11 +2460,11 @@ sysmalloc_mmap (INTERNAL_SIZE_T nb, size_t pagesize, int extra_flags, mstate av)
>      }
>  
>    /* update statistics */
> -  int new = atomic_exchange_and_add (&mp_.n_mmaps, 1) + 1;
> +  int new = atomic_fetch_add_relaxed (&mp_.n_mmaps, 1) + 1;
>    atomic_max (&mp_.max_n_mmaps, new);
>  
>    unsigned long sum;
> -  sum = atomic_exchange_and_add (&mp_.mmapped_mem, size) + size;
> +  sum = atomic_fetch_add_relaxed (&mp_.mmapped_mem, size) + size;
>    atomic_max (&mp_.max_mmapped_mem, sum);
>  
>    check_chunk (av, p);
> @@ -3084,7 +3084,7 @@ mremap_chunk (mchunkptr p, size_t new_size)
>    set_head (p, (new_size - offset) | IS_MMAPPED);
>  
>    INTERNAL_SIZE_T new;
> -  new = atomic_exchange_and_add (&mp_.mmapped_mem, new_size - size - offset)
> +  new = atomic_fetch_add_relaxed (&mp_.mmapped_mem, new_size - size - offset)
>          + new_size - size - offset;
>    atomic_max (&mp_.max_mmapped_mem, new);
>    return p;

Wrong patch, I think?

Thanks,
Florian