bitmap_set() performs a plain (non-atomic) read-modify-write on every
level of the bitmap tree:
g = *gp; /* READ */
g ^= ZU(1) << bit; /* MODIFY — thread-local copy */
*gp = g; /* WRITE BACK — no barrier, no CAS */
Two threads that reach bitmap_sfu() -> bitmap_set() concurrently on the
same slab bitmap — even for different bits that share a group word —
will clobber each other's write. The clobbered bit still looks free on
the next allocation; bitmap_sfu() selects it again; the second call to
bitmap_set() aborts on:
assert(!bitmap_get(bitmap, binfo, bit)); /* bitmap.h:220 */
or, once tree propagation begins for a newly-full group:
assert(g & (ZU(1) << (bit & BITMAP_GROUP_NBITS_MASK))); /* bitmap.h:237 */
Either assert calls abort() and produces the coredump reported in
issues #2875 and #2772.
The immediate callers (bin_malloc_with_fresh_slab,
bin_malloc_no_fresh_slab) already assert lock ownership, but
bin_slab_reg_alloc() itself had no such check, making it easy for new
call sites to silently bypass the requirement.
Fix:
- Thread tsdn_t *tsdn and bin_t *bin through bin_slab_reg_alloc() and
call malloc_mutex_assert_owner() as the first statement.
- Update both internal callers (bin_malloc_with_fresh_slab,
bin_malloc_no_fresh_slab) to pass the context they already hold.
- Document the locking contract in bin.h and the thread-safety
constraint in bitmap.h directly above bitmap_set().
Note: bin_slab_reg_alloc_batch() is left unchanged because it has one
legitimate unlocked caller (arena_fill_small_fresh) which operates on
freshly allocated slabs that are not yet visible to any other thread.
Its locking contract is now documented in bin.h.
Fixes#2875
The second expansion attempt in large_ralloc_no_move omitted the !
before large_ralloc_no_move_expand(), inverting the return value.
On expansion failure, the function falsely reported success, making
callers believe the allocation was expanded in-place when it was not.
On expansion success, the function falsely reported failure, causing
callers to unnecessarily allocate, copy, and free.
Add unit test that verifies the return value matches actual size change.
These functions had zero callers anywhere in the codebase:
- extent_commit_wrapper: wrapper never called, _impl used directly
- large_salloc: trivial wrapper never called
- tcache_gc_dalloc_new_event_wait: no header declaration, no callers
- tcache_gc_dalloc_postponed_event_wait: no header declaration, no callers
A few ways this consistency check can be improved:
* Print which conditions fail and associated values.
* Accumulate the result so that we can print all conditions that fail.
* Turn hpdata_assert_consistent() into a macro so, when it fails,
we can get line number where it's called from.
tsd_tcache_data_init() returns true on failure but its callers ignore
this return value, leaving the per-thread tcache in an uninitialized
state after a failure.
This change disables the tcache on an initialization failure and logs
an error message. If opt_abort is true, it will also abort.
New unit tests have been added to test tcache initialization failures.
This is a clean-up change that gives the bin functions implemented in
the area code a prefix of bin_ and moves them into the bin code.
To further decouple the bin code from the arena code, bin functions
that had taken an arena_t to check arena_is_auto now take an is_auto
parameter instead.
The static inline definition made more sense when these functions just
dispatched to a syscall wrapper. Since they acquired a retry loop, a
non-inline definition makes more sense.
Giving the advice MADV_DONTNEED to a range of virtual memory backed by
a transparent huge page already causes that range of virtual memory to
become backed by regular pages.
any future changes to the underlying data type for bin sizes
(such as upgrading from `uint16_t` to `uint32_t`) can be achieved
by modifying only the `cache_bin_sz_t` definition.
Signed-off-by: Xin Yang <yangxin.dev@bytedance.com>
The maximum allowed value for `nflush_batch` is
`CACHE_BIN_NFLUSH_BATCH_MAX`. However, `tcache_bin_flush_impl_small`
could potentially declare an array of `emap_batch_lookup_result_t`
of size `CACHE_BIN_NFLUSH_BATCH_MAX + 1`. leads to a `VARIABLE_ARRAY`
assertion failure, observed when `tcache_nslots_small_max` is
configured to 2048. This patch ensures the array size does not exceed
the allowed maximum.
Signed-off-by: Xin Yang <yangxin.dev@bytedance.com>
