Question about the spare chunk

[cee1]

2014-12-05 13:17 GMT+08:00 cee1 <fykcee1 at gmail.com>:
> Hi,
>
> Thanks for the reply!
>
> 2014-12-04 7:41 GMT+08:00 Jason Evans <jasone at canonware.com>:
>> On Dec 3, 2014, at 6:17 AM, cee1 <fykcee1 at gmail.com> wrote:
>>> Here the old spare chunk is replaced by the new one and dealloced.
>>>
>>> The deallocating process is done without the lock protection. In this
>>> lockless period, is it possible another arena_chunk_dealloc running in
>>> another thread replaces and deallocates ** our new spare chunk ** ?
>>> Which is still in the chunks_dirty tree and will be accessed by the
>>> purge process later.
>>
>> The chunk being passed to chunk_dealloc() has been completely dissociated from the arena prior to the malloc_mutex_unlock() call, so there's no way for it to be accessed by the arena again.  If another thread concurrently calls into arena_chunk_dealloc() to deallocate the current spare, then the same logic holds for that chunk deallocation.
>
> Could you describe this more detail?
>
> """   <--- code snippet in arena_chunk_dealloc()
>
> assert(arena_mapbits_dirty_get(chunk, map_bias) ==
> arena_mapbits_dirty_get(chunk, chunk_npages-1));
> /*
>  * Remove run from the runs_avail tree, so that the arena does not use
>  * it.
>  */
> arena_avail_remove(arena, chunk, map_bias, chunk_npages-map_bias, false, false);
>
> if (arena->spare != NULL) {
>     arena_chunk_t *spare = arena->spare;
>     arena->spare = chunk;
>
>     malloc_mutex_unlock(&arena->lock);
>     ...
> """
>
> What I understand:
> 1. The chunk may be whole dirty, i.e. n_dirty == 1.
>
> 2. After calling arena_avail_remove, the chunk may still be in
> arena->chunks_dirty

Sorry for didn't notice the chunk will be removed from
arena->chunks_dirty in arena_avail_remove():
"""
if (arena_mapbits_dirty_get(chunk, pageind) != 0) {
    arena->ndirty -= npages;
    chunk->ndirty -= npages;
}

if (chunk->ndirty != 0)
    arena_chunk_dirty_insert(&arena->chunks_dirty, chunk);
"""

Then the question becomes what case does the branch in
arena_chunk_purge() to catch:
"""
if (chunk == arena->spare) {
    assert(arena_mapbits_dirty_get(chunk, map_bias) != 0);
    assert(arena_mapbits_dirty_get(chunk, chunk_npages-1) != 0);
    arena_chunk_alloc(arena);
}
"""

The invocation path is
"arena_run_dalloc()/arena_maybe_purge()/arena_purge()
/arena_chunk_purge(arena, chunk, all)"

I notice:
1. The chunk parameter of arena_chunk_purge() is from "chunk =
arena_chunk_dirty_first(&arena->chunks_dirty)"
2. arena->chunks_dirty should not hold any reference to arena->spare

So the branch should always be skipped, am I right?



>
> BTW, in function arena_run_regind(), I notice a "plus 1" in
> """
> define SIZE_INV(s)     (((1U << SIZE_INV_SHIFT) / (s)) + 1)
> """
>
> Why we need this?



-- 
Regards,

- cee1