On 30.09.2016 15:47, Marek Olšák wrote:
On Fri, Sep 30, 2016 at 3:08 PM, Bas Nieuwenhuizen
<b...@basnieuwenhuizen.nl> wrote:
On Fri, Sep 30, 2016 at 2:13 PM, Marek Olšák <mar...@gmail.com> wrote:
intptr_t reads and writes aren't atomic. p_atomic_set and
p_atomic_read functions don't do anything for atomicity. See:
#define p_atomic_set(_v, _i) (*(_v) = (_i))
#define p_atomic_read(_v) (*(_v))
That implementation seems bogus to me, as the compiler sees none of
them as atomic and therefore the compiler can do strange stuff.
why are intptr_t reads/writes less atomic than int32_t? IIRC on x86_64
aligned 64-bit accesses are atomic, and on x86 intptr_t is just 32
bits.
I looked into a number of options for p_atomic_set/read and just sent
around a patch which (to my understanding) definitely guarantees
sufficient atomicity and memory ordering on GCC >= 4.7.
Without that patch (and so I suspect also on GCC < 4.7), the memory
accesses are still de facto atomic as Bas wrote. Furthermore, most of
the necessary ordering guarantees are established by the calls to
mtx_lock/unlock functions.
Without that patch (and also with that patch but with GCC < 4.7), there
is actually still the possibility that the write of
page->u.num_remaining in slab_destroy_child is moved to after the loop
over the page's elements. GCC doesn't actually do it in practice, but it
is a gap which we probably have to live with unless we introduce some
ugly workarounds.
Note that this is only ever a problem in the situation where an
allocation is freed with a different child pool than the one it was
allocated from. In other words, the new code is (as far as I can see)
only buggy in the case where the old code was even buggier.
For what it's worth, I'm going to use p_atomic_set also for
page->u.num_remaining. This is not strictly needed, since the
acquire/release on elt->owner already establishes the necessary ordering
already, but it should help clarity.
Do you agree with this plan?
Thanks,
Nicolai
Really? Thanks, I didn't know that.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
