[PATCH v2] malloc: Change mmap chunk layout
Cupertino Miranda
cupertino.miranda@oracle.com
Mon Aug 25 13:48:56 GMT 2025
Hi Wilco,
As discussed in patchwork meeting and following my previous review, here
is my:
Reviewed-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cheers,
Cupertino
On 21-08-2025 16:40, Cupertino Miranda wrote:
> Hi Wilco,
>
> Apologies for missing v1 of this patch.
>
> For what is worth, patch looks good to me.
> Brief inline comments.
>
> Cheers,
> Cupertino
>
> On 20-08-2025 18:29, Wilco Dijkstra wrote:
>> v2: Rebase for CI, use inline functions
>>
>> Change the mmap chunk layout to be identical to a normal chunk. This
>> makes it
>> safe for tcache to hold mmap chunks and simplifies size calculations
>> in memsize
>> and musable. Add mmap_base() and mmap_size() accessor functions to
>> simplify code.
>
>>
>> Passes regress, OK for commit?
>>
>> ---
>>
>> diff --git a/malloc/malloc-check.c b/malloc/malloc-check.c
>> index
>> 9532316a298c4b6ef779f3ac8ee7692d78d5e1db..40b215ea83f43fbc09dc5f303454ce516c9a6ae1 100644
>> --- a/malloc/malloc-check.c
>> +++ b/malloc/malloc-check.c
>> @@ -151,8 +151,8 @@ mem2chunk_check (void *mem, unsigned char **magic_p)
>> offset != 0x200 && offset != 0x400 && offset != 0x800 &&
>> offset != 0x1000 &&
>> offset < 0x2000) ||
>> !chunk_is_mmapped (p) || prev_inuse (p) ||
>> - ((((unsigned long) p - prev_size (p)) & page_mask) != 0) ||
>> - ((prev_size (p) + sz) & page_mask) != 0)
>> + ((((uintptr_t) mmap_base (p)) & page_mask) != 0) ||
>> + (mmap_size (p) & page_mask) != 0)
> Ok.
>> return NULL;
>> for (sz = CHUNK_HDR_SZ + memsize (p) - 1;
>> @@ -273,7 +273,6 @@ realloc_check (void *oldmem, size_t bytes)
>> __libc_lock_unlock (main_arena.mutex);
>> if (!oldp)
>> malloc_printerr ("realloc(): invalid pointer");
>> - const INTERNAL_SIZE_T oldsize = chunksize (oldp);
>> if (rb > PTRDIFF_MAX)
>> {
>> @@ -293,8 +292,8 @@ realloc_check (void *oldmem, size_t bytes)
>> else
>> #endif
>> {
>> - /* Note the extra SIZE_SZ overhead. */
>> - if (oldsize - SIZE_SZ >= chnb)
>> + size_t oldsize = memsize (oldp);
>> + if (oldsize >= rb)
>> newmem = oldmem; /* do nothing */
>> else
>> {
>> @@ -303,7 +302,7 @@ realloc_check (void *oldmem, size_t bytes)
>> newmem = _int_malloc (&main_arena, rb);
>> if (newmem)
>> {
>> - memcpy (newmem, oldmem, oldsize - CHUNK_HDR_SZ);
>> + memcpy (newmem, oldmem, oldsize);
>> munmap_chunk (oldp);
>> }
>> }
>> @@ -312,7 +311,7 @@ realloc_check (void *oldmem, size_t bytes)
>> else
>> {
>> top_check ();
>> - newmem = _int_realloc (&main_arena, oldp, oldsize, chnb);
>> + newmem = _int_realloc (&main_arena, oldp, chunksize (oldp), chnb);
> Ok. Operated on mem dimensions instead of chunk size.
>> }
>> DIAG_PUSH_NEEDS_COMMENT;
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index
>> e08873cad50ee9b05bda1eb3b50cef954ff7685a..f1cf392d7940ee00e227732acac1da8f3484e775 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -1458,7 +1458,7 @@ checked_request2size (size_t req) __nonnull (1)
>> #define memsize(p) \
>> (__MTAG_GRANULE_SIZE > SIZE_SZ && __glibc_unlikely (mtag_enabled) ? \
>> chunksize (p) - CHUNK_HDR_SZ : \
>> - chunksize (p) - CHUNK_HDR_SZ + (chunk_is_mmapped (p) ? 0 : SIZE_SZ))
>> + chunksize (p) - CHUNK_HDR_SZ + SIZE_SZ)
> Uniformed to the same size.
>> /* If memory tagging is enabled the layout changes to accommodate
>> the granule
>> size, this is wasteful for small allocations so not done by default.
>> @@ -1477,6 +1477,20 @@ tag_new_usable (void *ptr)
>> return ptr;
>> }
>> +/* Return pointer to mmap base from a chunk with IS_MMAPPED set. */
>> +static __always_inline void *
>> +mmap_base (mchunkptr p)
>> +{
>> + return (char *) p - prev_size (p);
>> +}
>> +
>> +/* Return total mmap size of a chunk with IS_MMAPPED set. */
>> +static __always_inline size_t
>> +mmap_size (mchunkptr p)
>> +{
>> + return prev_size (p) + chunksize (p) + CHUNK_HDR_SZ;
>> +}
>> +
> Ok.
>> /*
>> -------------------- Internal data structures --------------------
>> @@ -2105,7 +2119,7 @@ do_check_chunk (mstate av, mchunkptr p)
>> else
>> {
>> /* chunk is page-aligned */
>> - assert (((prev_size (p) + sz) & (GLRO (dl_pagesize) - 1)) == 0);
>> + assert ((mmap_size (p) & (GLRO (dl_pagesize) - 1)) == 0);
>> /* mem is aligned */
>> assert (!misaligned_chunk (p));
>> }
>> @@ -2405,14 +2419,14 @@ do_check_malloc_state (mstate av)
>> /* Allocate a mmap chunk - used for large block sizes or as a fallback.
>> Round up size to nearest page. Add padding if MALLOC_ALIGNMENT is
>> - larger than CHUNK_HDR_SZ. Add SIZE_SZ at the end since there is no
>> - following chunk whose prev_size field could be used. */
>> + larger than CHUNK_HDR_SZ. Add CHUNK_HDR_SZ at the end so that mmap
>> + chunks have the same layout as regular chunks. */
>> static void *
>> sysmalloc_mmap (INTERNAL_SIZE_T nb, size_t pagesize, int extra_flags)
>> {
>> size_t padding = MALLOC_ALIGNMENT - CHUNK_HDR_SZ;
>> - size_t size = ALIGN_UP (nb + padding + SIZE_SZ, pagesize);
>> + size_t size = ALIGN_UP (nb + padding + CHUNK_HDR_SZ, pagesize);
>> char *mm = (char *) MMAP (NULL, size,
>> mtag_mmap_flags | PROT_READ | PROT_WRITE,
>> @@ -2427,7 +2441,7 @@ sysmalloc_mmap (INTERNAL_SIZE_T nb, size_t
>> pagesize, int extra_flags)
>> /* Store offset to start of mmap in prev_size. */
>> mchunkptr p = (mchunkptr) (mm + padding);
>> set_prev_size (p, padding);
>> - set_head (p, (size - padding) | IS_MMAPPED);
>> + set_head (p, (size - padding - CHUNK_HDR_SZ) | IS_MMAPPED);
>> /* update statistics */
>> int new = atomic_fetch_add_relaxed (&mp_.n_mmaps, 1) + 1;
>> @@ -2977,13 +2991,12 @@ static void
>> munmap_chunk (mchunkptr p)
>> {
>> size_t pagesize = GLRO (dl_pagesize);
>> - INTERNAL_SIZE_T size = chunksize (p);
>> assert (chunk_is_mmapped (p));
>> uintptr_t mem = (uintptr_t) chunk2mem (p);
>> - uintptr_t block = (uintptr_t) p - prev_size (p);
>> - size_t total_size = prev_size (p) + size;
>> + uintptr_t block = (uintptr_t) mmap_base (p);
>> + size_t total_size = mmap_size (p);
>> /* Unfortunately we have to do the compilers job by hand here.
>> Normally
>> we would test BLOCK and TOTAL-SIZE separately for compliance
>> with the
>> page size. But gcc does not recognize the optimization
>> possibility
>> @@ -3014,15 +3027,15 @@ mremap_chunk (mchunkptr p, size_t new_size)
>> assert (chunk_is_mmapped (p));
>> - uintptr_t block = (uintptr_t) p - offset;
>> + uintptr_t block = (uintptr_t) mmap_base (p);
>> uintptr_t mem = (uintptr_t) chunk2mem(p);
>> - size_t total_size = offset + size;
>> + size_t total_size = mmap_size (p);
>> if (__glibc_unlikely ((block | total_size) & (pagesize - 1)) != 0
>> || __glibc_unlikely (!powerof2 (mem & (pagesize - 1))))
>> malloc_printerr("mremap_chunk(): invalid pointer");
>> - /* Note the extra SIZE_SZ overhead as in mmap_chunk(). */
>> - new_size = ALIGN_UP (new_size + offset + SIZE_SZ, pagesize);
>> + /* Note the extra CHUNK_HDR_SZ overhead as in mmap_chunk(). */
>> + new_size = ALIGN_UP (new_size + offset + CHUNK_HDR_SZ, pagesize);
> Ok.
>> /* No need to remap if the number of pages does not change. */
>> if (total_size == new_size)
>> @@ -3041,7 +3054,7 @@ mremap_chunk (mchunkptr p, size_t new_size)
>> assert (!misaligned_chunk (p));
>> assert (prev_size (p) == offset);
>> - set_head (p, (new_size - offset) | IS_MMAPPED);
>> + set_head (p, (new_size - offset - CHUNK_HDR_SZ) | IS_MMAPPED);
> Ok.
>> INTERNAL_SIZE_T new;
>> new = atomic_fetch_add_relaxed (&mp_.mmapped_mem, new_size - size
>> - offset)
>> @@ -3340,11 +3353,6 @@ tcache_init (void)
>> if (tcache_shutting_down)
>> return;
>> - /* Check minimum mmap chunk is larger than max tcache size. This
>> means
>> - mmap chunks with their different layout are never added to
>> tcache. */
>> - if (MAX_TCACHE_SMALL_SIZE >= GLRO (dl_pagesize) / 2)
>> - malloc_printerr ("max tcache size too large");
>> -
> Good! ;-)
>> size_t bytes = sizeof (tcache_perthread_struct);
>> tcache = (tcache_perthread_struct *) __libc_malloc2 (bytes);
>> @@ -3492,7 +3500,6 @@ __libc_free (void *mem)
>> {
>> tc_idx = large_csize2tidx (size);
>> if (size >= MINSIZE
>> - && !chunk_is_mmapped (p)
>> && __glibc_likely (tcache->num_slots[tc_idx] != 0))
>> return tcache_put_large (p, tc_idx);
>> }
>> @@ -5279,7 +5286,7 @@ musable (void *mem)
>> mchunkptr p = mem2chunk (mem);
>> if (chunk_is_mmapped (p))
>> - return chunksize (p) - CHUNK_HDR_SZ;
>> + return memsize (p);
> Ok.
>> else if (inuse (p))
>> return memsize (p);
>>
>
More information about the Libc-alpha
mailing list