Skip to content

Commit fee8acc

Browse files
committed
mem: the pre-fork pkg arena must not be hugetlb - a child's COW there is a SIGBUS
Every forked child inherits the attendant's pkg arena copy-on-write and keeps it for life (module state parsed pre-fork lives there). While that arena sat on MAP_HUGETLB, a child's first write into any of its pages needed a fresh huge page - no 4K fallback inside a hugetlb VMA, and a forked child holds no reservation on its parent's mapping - so with the pool momentarily empty the fault was a silent SIGBUS. Startup exposed it deterministically: the last no-script child (TCP main) freed the inherited route AST before swapping to its own arena, and died with 'failed to initialize child process N / cannot fork tcp main process' the moment the pool ran short (08-07 slot 52 on x86, again on an ARM box with a small pool). The 08-07 ordering fix made that path free against the right arena; it could not make it pool-independent. Two changes, either sufficient, both kept: - HG_INIT_INHERITED: hg_malloc_init() takes a flags word; the pre-fork pkg arena passes it and hg_mem_reserve() starts its ladder at THP. THP COW splits the huge PMD and copies one 4K page - it cannot SIGBUS, and it cuts every child's COW cost from 2 MB per touched page to 4 KB. Per-child arenas are never inherited and keep the full ladder. - internal_fork_child_setup() no longer walks the inherited route AST with pkg_free under HG pkg: the child abandons that arena wholesale, so the frees bought nothing and were the one pre-swap writer. Proven fail-first on a 16-child rig (-m 32 -M 8): pool 40 and 60 - unpatched dies at child 14 = TCP main, strace shows SIGBUS BUS_ADRERR with si_addr inside the attendant's 8 MB MAP_PRIVATE|MAP_HUGETLB mapping; patched starts all 17 processes on the same pools, and more children land on hugetlb since the pool no longer feeds the parent's arena or COW pages. Pool 0 starts either way, as the mechanism predicts.
1 parent e628a56 commit fee8acc

5 files changed

Lines changed: 90 additions & 32 deletions

File tree

‎mem/hg_malloc.c‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ static void hg_exclude_from_core(void *base, unsigned long size)
260260
* between processes through the shared gpool.
261261
*/
262262
static void *hg_mem_reserve(unsigned long size, unsigned long *cap,
263-
enum hg_mem_tier *tier, unsigned long *locked_mb, int shared)
263+
enum hg_mem_tier *tier, unsigned long *locked_mb, int shared,
264+
int inherited)
264265
{
265266
unsigned long asize = HG_HPS_ROUND(size);
266267
unsigned long csize = HG_HPS_ROUND(*cap < size ? size : *cap);
@@ -298,9 +299,17 @@ static void *hg_mem_reserve(unsigned long size, unsigned long *cap,
298299
* trade than simply not being able to grow. A cap-less tier-1 arena is
299300
* what v2 shipped; losing the tier is a real regression.
300301
*/
301-
p = mmap(NULL, csize, PROT_READ|PROT_WRITE,
302+
/*
303+
* ...except for the arena children inherit copy-on-write (the pre-fork
304+
* pkg arena, HG_INIT_INHERITED): a child's write into an inherited
305+
* hugetlb page needs a fresh huge page with no 4K fallback and no
306+
* reservation behind it - an empty pool at that instant is a SIGBUS.
307+
* That arena starts the ladder at THP, whose COW splits to 4K pages
308+
* instead. See the flag's comment in hg_malloc.h.
309+
*/
310+
p = inherited ? MAP_FAILED : mmap(NULL, csize, PROT_READ|PROT_WRITE,
302311
vis|MAP_ANONYMOUS|MAP_HUGETLB, -1, 0);
303-
if (p == MAP_FAILED && csize > asize) {
312+
if (p == MAP_FAILED && !inherited && csize > asize) {
304313
p = mmap(NULL, asize, PROT_READ|PROT_WRITE,
305314
vis|MAP_ANONYMOUS|MAP_HUGETLB, -1, 0);
306315
if (p != MAP_FAILED) {
@@ -816,7 +825,7 @@ static void hg_arena_reg_del(struct hg_block *hb)
816825
* (it cannot: HG_MALLOC IS what those macros dispatch to when selected).
817826
*/
818827
struct hg_block *hg_malloc_init(unsigned long size, char *name, int shared,
819-
const char *proc_desc)
828+
const char *proc_desc, unsigned int flags)
820829
{
821830
enum hg_mem_tier tier;
822831
unsigned long locked_mb;
@@ -842,7 +851,8 @@ struct hg_block *hg_malloc_init(unsigned long size, char *name, int shared,
842851
cap = hg_pkg_cap_bytes;
843852
else
844853
cap = 0;
845-
base = hg_mem_reserve(size, &cap, &tier, &locked_mb, shared);
854+
base = hg_mem_reserve(size, &cap, &tier, &locked_mb, shared,
855+
(flags & HG_INIT_INHERITED) != 0);
846856
if (!base) {
847857
LM_ERR("failed to reserve %lu bytes for %s HG_MALLOC arena\n",
848858
size, name);
@@ -931,8 +941,11 @@ struct hg_block *hg_malloc_init(unsigned long size, char *name, int shared,
931941
name, proc_desc, size >> 20, hg_mem_tier_str(tier), locked_mb);
932942
else
933943
LM_NOTICE("%s " HG_MALLOC_NAME " arena: %lu MB on %s, %lu MB "
934-
"pinned from swapping\n",
935-
name, size >> 20, hg_mem_tier_str(tier), locked_mb);
944+
"pinned from swapping%s\n",
945+
name, size >> 20, hg_mem_tier_str(tier), locked_mb,
946+
(flags & HG_INIT_INHERITED) ?
947+
" (pre-fork arena, inherited copy-on-write by every child: "
948+
"hugetlb deliberately skipped, its COW cannot fall back)" : "");
936949
if (hb->hcap > hb->hsize)
937950
LM_NOTICE("%s arena can grow to %lu MB (%lu MB headroom "
938951
"reserved, uncommitted)\n", name, hb->hcap >> 20,

‎mem/hg_malloc.h‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,9 +713,24 @@ static inline unsigned long hg_leaf_of(const struct hg_block *hb, const void *p)
713713
/* @shared: 1 for shm/shm_dbg (MAP_SHARED - one arena for every worker),
714714
* 0 for pkg (MAP_PRIVATE - each forked worker gets its own copy-on-write
715715
* arena, lock and free pools). See hg_mem_reserve() in hg_malloc.c for why
716-
* getting this wrong for pkg is a correctness AND a performance bug. */
716+
* getting this wrong for pkg is a correctness AND a performance bug.
717+
*
718+
* @flags: HG_INIT_INHERITED marks the ONE arena that outlives fork() as a
719+
* copy-on-write inheritance: the pre-fork (attendant) pkg arena. Every
720+
* child holds a private COW view of it for life - it reads module state
721+
* the parent parsed into it, and any write it makes there is a COW fault.
722+
* Such an arena must never be hugetlb-backed: a COW fault inside a
723+
* hugetlb VMA can only be satisfied by a huge page (no 4K fallback), and
724+
* a forked child holds no reservation on its parent's mapping, so with
725+
* the pool momentarily empty the fault is a SIGBUS - silent, at fork
726+
* time (measured: it is exactly how the last no-script child, TCP main,
727+
* died at startup on a short pool). THP has the fallback: a COW fault
728+
* splits the huge PMD and copies one 4K page. Per-child arenas are never
729+
* inherited (children do not fork) and keep the full ladder. */
730+
#define HG_INIT_INHERITED (1U << 0)
731+
717732
struct hg_block *hg_malloc_init(unsigned long size, char *name, int shared,
718-
const char *proc_desc);
733+
const char *proc_desc, unsigned int flags);
719734
void hg_malloc_destroy(struct hg_block *hb);
720735

721736
/*

‎mem/mem.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,17 @@ int init_pkg_mallocs(void)
8989
* leak that block (never freed, never used) - across dozens of
9090
* worker processes that adds up, so the call is skipped for it
9191
* rather than allocated-and-ignored.
92+
*
93+
* HG_INIT_INHERITED: this is the ONE arena every forked child
94+
* inherits copy-on-write (pt.c hands each child a fresh arena of its
95+
* own, but the parent's stays mapped in the child, holding all the
96+
* module state parsed pre-fork). It must not sit on hugetlb - a
97+
* child's COW fault there has no 4K fallback and no reservation, so
98+
* an empty pool at fork is a silent SIGBUS. It starts on THP instead.
9299
*/
93100
#ifdef INLINE_ALLOC
94101
#if defined HG_MALLOC
95-
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL);
102+
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL, HG_INIT_INHERITED);
96103
#else
97104
mem_pool = malloc(pkg_mem_size);
98105
if (!mem_pool) {
@@ -170,7 +177,7 @@ int init_pkg_mallocs(void)
170177
#endif
171178
#ifdef HG_MALLOC
172179
case MM_HG_MALLOC:
173-
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL);
180+
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL, HG_INIT_INHERITED);
174181
gen_pkg_malloc = (osips_block_malloc_f)hg_malloc;
175182
gen_pkg_realloc = (osips_block_realloc_f)hg_realloc;
176183
gen_pkg_free = (osips_block_free_f)hg_free;
@@ -235,7 +242,7 @@ int init_pkg_mallocs(void)
235242
#endif
236243
#ifdef HG_MALLOC
237244
case MM_HG_MALLOC_DBG:
238-
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL);
245+
mem_block = hg_malloc_init(pkg_mem_size, "pkg", 0, NULL, HG_INIT_INHERITED);
239246
gen_pkg_malloc = (osips_block_malloc_f)hg_malloc_dbg;
240247
gen_pkg_realloc = (osips_block_realloc_f)hg_realloc_dbg;
241248
gen_pkg_free = (osips_block_free_f)hg_free_dbg;

‎mem/shm_mem.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ int shm_mem_init_mallocs(void* mempool, unsigned long pool_size,int idx)
298298
/* ignores mempool/pool_size as an address - HG_MALLOC does its own
299299
* reservation, pool_size is reused only as the size to request, see
300300
* the shm_mem_init() caller-side skip of shm_getmem() for this case */
301-
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL);
301+
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL, 0);
302302
#endif
303303
#else
304304

@@ -528,7 +528,7 @@ int shm_mem_init_mallocs(void* mempool, unsigned long pool_size,int idx)
528528
case MM_HG_MALLOC:
529529
/* ignores mempool - HG_MALLOC does its own reservation; see the
530530
* shm_mem_init() caller-side skip of shm_getmem() for this case */
531-
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL);
531+
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL, 0);
532532
gen_shm_malloc = (osips_block_malloc_f)hg_malloc;
533533
gen_shm_malloc_unsafe = (osips_block_malloc_f)hg_malloc;
534534
gen_shm_realloc = (osips_block_realloc_f)hg_realloc;
@@ -605,7 +605,7 @@ int shm_mem_init_mallocs(void* mempool, unsigned long pool_size,int idx)
605605
#endif
606606
#ifdef HG_MALLOC
607607
case MM_HG_MALLOC_DBG:
608-
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL);
608+
shm_block = hg_malloc_init(pool_size, "shm", 1, NULL, 0);
609609
gen_shm_malloc = (osips_block_malloc_f)hg_malloc_dbg;
610610
gen_shm_malloc_unsafe = (osips_block_malloc_f)hg_malloc_dbg;
611611
gen_shm_realloc = (osips_block_realloc_f)hg_realloc_dbg;
@@ -811,7 +811,7 @@ int shm_dbg_mem_init_mallocs(void* mempool, unsigned long pool_size)
811811
case MM_HG_MALLOC:
812812
case MM_HG_MALLOC_DBG:
813813
/* ignores mempool, same reasoning as shm_mem_init_mallocs() */
814-
shm_dbg_block = hg_malloc_init(pool_size, "shm_dbg", 1, NULL);
814+
shm_dbg_block = hg_malloc_init(pool_size, "shm_dbg", 1, NULL, 0);
815815
break;
816816
#endif
817817
default:

‎pt.c‎

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,29 @@ static int internal_fork_child_setup(const struct internal_fork_params *ifpp)
556556

557557
/* free the script if not needed */
558558
if (!(ifpp->flags & OSS_PROC_NEEDS_SCRIPT) && sroutes) {
559+
#if defined(PKG_MALLOC) && defined(HG_MALLOC)
560+
/*
561+
* Under HG_MALLOC this child is about to abandon the whole pkg
562+
* arena it inherited from the parent (internal_fork() swaps in
563+
* a fresh, private one right after this handler chain), so
564+
* freeing the route AST cell by cell buys nothing - and it was
565+
* the one thing writing into the inherited arena before the
566+
* swap: every such write is a copy-on-write fault against the
567+
* parent's mapping, and while that mapping was hugetlb-backed
568+
* an empty pool turned the very first one into a SIGBUS (how
569+
* TCP main, the last no-script child, died at startup on a
570+
* short pool). The parent's arena no longer sits on hugetlb
571+
* (HG_INIT_INHERITED), so this is not load-bearing for safety
572+
* any more - it just spares every no-script child a private
573+
* 4K copy of each AST page it would otherwise dirty. Dropping
574+
* the pointer is all the child needs.
575+
*/
576+
if (mem_allocator_pkg == MM_HG_MALLOC ||
577+
mem_allocator_pkg == MM_HG_MALLOC_DBG) {
578+
sroutes = NULL;
579+
return 0;
580+
}
581+
#endif
559582
free_route_lists(sroutes);
560583
sroutes = NULL;
561584
}
@@ -726,24 +749,24 @@ int internal_fork(const struct internal_fork_params *ifpp)
726749
* specific process - exactly the semantics wanted here, just
727750
* never previously invoked per child.
728751
*
729-
* MUST run after the post-fork handler chain below, not before:
730-
* internal_fork_child_setup() (the default handler) calls
731-
* free_route_lists(), which pkg_frees the route AST this child
732-
* inherited from the parent pre-fork. Those pointers only make
733-
* sense against the ORIGINAL (COW-inherited) arena. Swapping
734-
* mem_block to a fresh arena before that handler runs made it
735-
* free foreign pointers through the new arena's bookkeeping -
736-
* silent cross-arena corruption at best, and a SIGBUS at worst
737-
* (the COW page-fault into the abandoned old arena has nothing
738-
* left to fall back on once the hugetlb pool is tight, which is
739-
* exactly what exposed this). Running the swap here instead
740-
* lets free_route_lists() finish its cleanup against the still-
741-
* correct original arena first; only then is it abandoned - the
742-
* parent's inherited mapping is left unmapped-but-unreferenced,
743-
* and this child never touches it again.
752+
* Runs after the post-fork handler chain, so anything a handler
753+
* frees is still resolved against the arena it was allocated
754+
* from (the inherited one). Two guarantees back this up now,
755+
* neither of which depends on the hugetlb pool having a page
756+
* left at fork time:
757+
* - internal_fork_child_setup() no longer walks the route AST
758+
* with pkg_free under HG (see there): core makes NO writes
759+
* into the inherited arena before the swap;
760+
* - the inherited arena is not hugetlb-backed
761+
* (HG_INIT_INHERITED in mem.c): should anything ever write
762+
* into it - now or later in the child's life - the COW is
763+
* an ordinary page fault with a 4K fallback, not a SIGBUS.
764+
* The parent's mapping stays in this child, unreferenced; the
765+
* child reads parent-parsed module state through it and never
766+
* allocates from it again.
744767
*/
745768
struct hg_block *child_pkg =
746-
hg_malloc_init(pkg_mem_size, "pkg", 0, ifpp->proc_desc);
769+
hg_malloc_init(pkg_mem_size, "pkg", 0, ifpp->proc_desc, 0);
747770
if (!child_pkg) {
748771
LM_CRIT("failed to init this child's own pkg memory "
749772
"(%lu bytes)\n", pkg_mem_size);

0 commit comments

Comments
 (0)