[Mesa-dev] [PATCH v2] util/slab: re-design to allow migration between pools (v2)
nhaehnle at gmail.com
Tue Oct 4 14:30:12 UTC 2016
On 30.09.2016 15:47, Marek Olšák wrote:
> On Fri, Sep 30, 2016 at 3:08 PM, Bas Nieuwenhuizen
> <bas at basnieuwenhuizen.nl> wrote:
>> On Fri, Sep 30, 2016 at 2:13 PM, Marek Olšák <maraeo at 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
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
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
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?
> Really? Thanks, I didn't know that.
More information about the mesa-dev