From 2bf27ec51e331371412576a9a9a67353109109ad Mon Sep 17 00:00:00 2001 From: Lukas Larsson Date: Thu, 31 Jan 2019 10:49:03 +0100 Subject: erts: Add ERL_NODE_BOOKKEEP to node tables refc --- erts/emulator/beam/beam_load.c | 3 +- erts/emulator/beam/bif.c | 19 +++++---- erts/emulator/beam/copy.c | 6 +-- erts/emulator/beam/erl_db_util.c | 2 +- erts/emulator/beam/erl_gc.c | 24 ++++++++++-- erts/emulator/beam/erl_message.c | 2 +- erts/emulator/beam/erl_node_tables.c | 41 ++++++++++++++++---- erts/emulator/beam/erl_node_tables.h | 69 +++++++++++++++++++++++++++++++-- erts/emulator/beam/erl_proc_sig_queue.c | 5 ++- erts/emulator/beam/external.c | 10 ++--- erts/emulator/beam/utils.c | 2 +- 11 files changed, 147 insertions(+), 36 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/beam_load.c b/erts/emulator/beam/beam_load.c index 7ff55e8927..e9e294cd59 100644 --- a/erts/emulator/beam/beam_load.c +++ b/erts/emulator/beam/beam_load.c @@ -6108,7 +6108,8 @@ erts_release_literal_area(ErtsLiteralArea* literal_area) } default: ASSERT(is_external_header(oh->thing_word)); - erts_deref_node_entry(((ExternalThing*)oh)->node); + erts_deref_node_entry(((ExternalThing*)oh)->node, + make_boxed(&oh->thing_word)); } oh = oh->next; } diff --git a/erts/emulator/beam/bif.c b/erts/emulator/beam/bif.c index 3cf0a02679..a4c7af9a24 100644 --- a/erts/emulator/beam/bif.c +++ b/erts/emulator/beam/bif.c @@ -4063,10 +4063,12 @@ BIF_RETTYPE list_to_pid_1(BIF_ALIST_1) if (is_nil(dep->cid)) goto bad; - enp = erts_find_or_insert_node(dep->sysname, dep->creation); + etp = (ExternalThing *) HAlloc(BIF_P, EXTERNAL_THING_HEAD_SIZE + 1); + + enp = erts_find_or_insert_node(dep->sysname, dep->creation, + make_boxed(&etp->header)); ASSERT(enp != erts_this_node); - etp = (ExternalThing *) HAlloc(BIF_P, EXTERNAL_THING_HEAD_SIZE + 1); etp->header = make_external_pid_header(1); etp->next = MSO(BIF_P).first; etp->node = enp; @@ -4130,10 +4132,11 @@ BIF_RETTYPE list_to_port_1(BIF_ALIST_1) if (is_nil(dep->cid)) goto bad; - enp = erts_find_or_insert_node(dep->sysname, dep->creation); + etp = (ExternalThing *) HAlloc(BIF_P, EXTERNAL_THING_HEAD_SIZE + 1); + enp = erts_find_or_insert_node(dep->sysname, dep->creation, + make_boxed(&etp->header)); ASSERT(enp != erts_this_node); - etp = (ExternalThing *) HAlloc(BIF_P, EXTERNAL_THING_HEAD_SIZE + 1); etp->header = make_external_port_header(1); etp->next = MSO(BIF_P).first; etp->node = enp; @@ -4236,9 +4239,6 @@ BIF_RETTYPE list_to_ref_1(BIF_ALIST_1) if (is_nil(dep->cid)) goto bad; - enp = erts_find_or_insert_node(dep->sysname, dep->creation); - ASSERT(enp != erts_this_node); - hsz = EXTERNAL_THING_HEAD_SIZE; #if defined(ARCH_64) hsz += n/2 + 1; @@ -4247,6 +4247,11 @@ BIF_RETTYPE list_to_ref_1(BIF_ALIST_1) #endif etp = (ExternalThing *) HAlloc(BIF_P, hsz); + + enp = erts_find_or_insert_node(dep->sysname, dep->creation, + make_boxed(&etp->header)); + ASSERT(enp != erts_this_node); + etp->header = make_external_ref_header(n/2); etp->next = BIF_P->off_heap.first; etp->node = enp; diff --git a/erts/emulator/beam/copy.c b/erts/emulator/beam/copy.c index e7bd046e18..db74b06cc5 100644 --- a/erts/emulator/beam/copy.c +++ b/erts/emulator/beam/copy.c @@ -854,7 +854,7 @@ Eterm copy_struct_x(Eterm obj, Uint sz, Eterm** hpp, ErlOffHeap* off_heap, Uint case EXTERNAL_REF_SUBTAG: { ExternalThing *etp = (ExternalThing *) objp; - erts_refc_inc(&etp->node->refc, 2); + erts_ref_node_entry(etp->node, 2, make_boxed(htop)); } L_off_heap_node_container_common: { @@ -1660,7 +1660,7 @@ Uint copy_shared_perform(Eterm obj, Uint size, erts_shcopy_t *info, case EXTERNAL_REF_SUBTAG: { ExternalThing *etp = (ExternalThing *) ptr; - erts_refc_inc(&etp->node->refc, 2); + erts_ref_node_entry(etp->node, 2, make_boxed(hp)); } off_heap_node_container_common: { @@ -1866,7 +1866,7 @@ Eterm copy_shallow(Eterm* ERTS_RESTRICT ptr, Uint sz, Eterm** hpp, case EXTERNAL_REF_SUBTAG: { ExternalThing* etp = (ExternalThing *) (tp-1); - erts_refc_inc(&etp->node->refc, 2); + erts_ref_node_entry(etp->node, 2, make_boxed(hp-1)); } off_heap_common: { diff --git a/erts/emulator/beam/erl_db_util.c b/erts/emulator/beam/erl_db_util.c index ec7442aeeb..1ea7074d21 100644 --- a/erts/emulator/beam/erl_db_util.c +++ b/erts/emulator/beam/erl_db_util.c @@ -3312,7 +3312,7 @@ void db_cleanup_offheap_comp(DbTerm* obj) default: ASSERT(is_external_header(u.hdr->thing_word)); ASSERT(u.pb != &tmp); - erts_deref_node_entry(u.ext->node); + erts_deref_node_entry(u.ext->node, make_boxed(u.ep)); break; } } diff --git a/erts/emulator/beam/erl_gc.c b/erts/emulator/beam/erl_gc.c index 3a50b294d1..9317850d96 100644 --- a/erts/emulator/beam/erl_gc.c +++ b/erts/emulator/beam/erl_gc.c @@ -1303,7 +1303,8 @@ erts_garbage_collect_literals(Process* p, Eterm* literals, ExternalThing *etp; ASSERT(is_external_header(ptr->thing_word)); etp = (ExternalThing *) ptr; - erts_refc_inc(&etp->node->refc, 1); + erts_ref_node_entry(etp->node, 1, + make_boxed(&oh->thing_word)); break; } } @@ -2836,7 +2837,11 @@ sweep_off_heap(Process *p, int fullsweep) while (ptr) { if (IS_MOVED_BOXED(ptr->thing_word)) { ASSERT(!ErtsInArea(ptr, oheap, oheap_sz)); - *prev = ptr = (struct erl_off_heap_header*) boxed_val(ptr->thing_word); + if (is_external_header(((struct erl_off_heap_header*) boxed_val(ptr->thing_word))->thing_word)) + erts_node_bookkeep(((ExternalThing*)ptr)->node, + make_boxed(&ptr->thing_word), + ERL_NODE_DEC); + *prev = ptr = (struct erl_off_heap_header*) boxed_val(ptr->thing_word); ASSERT(!IS_MOVED_BOXED(ptr->thing_word)); switch (ptr->thing_word) { case HEADER_PROC_BIN: { @@ -2863,6 +2868,11 @@ sweep_off_heap(Process *p, int fullsweep) /* fall through... */ } default: + if (is_external_header(ptr->thing_word)) { + erts_node_bookkeep(((ExternalThing*)ptr)->node, + make_boxed(&ptr->thing_word), + ERL_NODE_INC); + } prev = &ptr->next; ptr = ptr->next; } @@ -2896,7 +2906,8 @@ sweep_off_heap(Process *p, int fullsweep) } default: ASSERT(is_external_header(ptr->thing_word)); - erts_deref_node_entry(((ExternalThing*)ptr)->node); + erts_deref_node_entry(((ExternalThing*)ptr)->node, + make_boxed(&ptr->thing_word)); } *prev = ptr = ptr->next; } @@ -3029,6 +3040,13 @@ offset_heap(Eterm* hp, Uint sz, Sint offs, char* area, Uint area_size) { struct erl_off_heap_header* oh = (struct erl_off_heap_header*) hp; + if (is_external_header(oh->thing_word)) { + erts_node_bookkeep(((ExternalThing*)oh)->node, + make_boxed(((Eterm*)oh)-offs), ERL_NODE_DEC); + erts_node_bookkeep(((ExternalThing*)oh)->node, + make_boxed((Eterm*)oh), ERL_NODE_INC); + } + if (ErtsInArea(oh->next, area, area_size)) { Eterm** uptr = (Eterm **) (void *) &oh->next; *uptr += offs; /* Patch the mso chain */ diff --git a/erts/emulator/beam/erl_message.c b/erts/emulator/beam/erl_message.c index e12a51a0ae..9d40754d2d 100644 --- a/erts/emulator/beam/erl_message.c +++ b/erts/emulator/beam/erl_message.c @@ -181,7 +181,7 @@ erts_cleanup_offheap(ErlOffHeap *offheap) break; default: ASSERT(is_external_header(u.hdr->thing_word)); - erts_deref_node_entry(u.ext->node); + erts_deref_node_entry(u.ext->node, make_boxed(u.ep)); break; } } diff --git a/erts/emulator/beam/erl_node_tables.c b/erts/emulator/beam/erl_node_tables.c index b301e5d56a..60c3be3223 100644 --- a/erts/emulator/beam/erl_node_tables.c +++ b/erts/emulator/beam/erl_node_tables.c @@ -817,11 +817,16 @@ node_table_alloc(void *venp_tmpl) node_entries++; - erts_refc_init(&enp->refc, -1); + erts_init_node_entry(enp, -1); enp->creation = ((ErlNode *) venp_tmpl)->creation; enp->sysname = ((ErlNode *) venp_tmpl)->sysname; enp->dist_entry = erts_find_or_insert_dist_entry(((ErlNode *) venp_tmpl)->sysname); +#ifdef ERL_NODE_BOOKKEEP + erts_atomic_init_nob(&enp->slot, 0); + sys_memzero(enp->books, sizeof(struct erl_node_bookkeeping) * 1024); +#endif + return (void *) enp; } @@ -874,7 +879,7 @@ erts_node_table_info(fmtfn_t to, void *to_arg) } -ErlNode *erts_find_or_insert_node(Eterm sysname, Uint32 creation) +ErlNode *erts_find_or_insert_node(Eterm sysname, Uint32 creation, Eterm book) { ErlNode *res; ErlNode ne; @@ -884,9 +889,9 @@ ErlNode *erts_find_or_insert_node(Eterm sysname, Uint32 creation) erts_rwmtx_rlock(&erts_node_table_rwmtx); res = hash_get(&erts_node_table, (void *) &ne); if (res && res != erts_this_node) { - erts_aint_t refc = erts_refc_inctest(&res->refc, 0); + erts_aint_t refc = erts_ref_node_entry(res, 0, book); if (refc < 2) /* New or pending delete */ - erts_refc_inc(&res->refc, 1); + erts_ref_node_entry(res, 1, THE_NON_VALUE); } erts_rwmtx_runlock(&erts_node_table_rwmtx); if (res) @@ -896,9 +901,9 @@ ErlNode *erts_find_or_insert_node(Eterm sysname, Uint32 creation) res = hash_put(&erts_node_table, (void *) &ne); ASSERT(res); if (res != erts_this_node) { - erts_aint_t refc = erts_refc_inctest(&res->refc, 0); + erts_aint_t refc = erts_ref_node_entry(res, 0, book); if (refc < 2) /* New or pending delete */ - erts_refc_inc(&res->refc, 1); + erts_ref_node_entry(res, 1, THE_NON_VALUE); } erts_rwmtx_rwunlock(&erts_node_table_rwmtx); return res; @@ -925,6 +930,7 @@ static void try_delete_node(void *venp) * * If refc > 0, the entry is in use. Keep the entry. */ + erts_node_bookkeep(enp, THE_NON_VALUE, ERL_NODE_DEC); refc = erts_refc_dectest(&enp->refc, -1); if (refc == -1) (void) hash_erase(&erts_node_table, (void *) enp); @@ -1022,7 +1028,7 @@ erts_set_this_node(Eterm sysname, Uint creation) erts_deref_dist_entry(erts_this_dist_entry); erts_this_node = NULL; /* to make sure refc is bumped for this node */ - erts_this_node = erts_find_or_insert_node(sysname, creation); + erts_this_node = erts_find_or_insert_node(sysname, creation, THE_NON_VALUE); erts_this_dist_entry = erts_this_node->dist_entry; erts_ref_dist_entry(erts_this_dist_entry); @@ -1091,7 +1097,7 @@ void erts_init_node_tables(int dd_sec) node_tmpl.creation = 0; erts_this_node = hash_put(&erts_node_table, &node_tmpl); /* +1 for erts_this_node */ - erts_refc_init(&erts_this_node->refc, 1); + erts_init_node_entry(erts_this_node, 1); ASSERT(erts_this_node->dist_entry != NULL); erts_this_dist_entry = erts_this_node->dist_entry; @@ -1844,6 +1850,25 @@ insert_sig_ext(ErtsDistExternal *edep, void *arg) insert_dist_entry(edep->dep, SIGNAL_REF, proc->common.id, 0); } +#ifdef ERL_NODE_BOOKKEEP +void +erts_node_bookkeep(ErlNode *np, Eterm term, int what) +{ + erts_aint_t slot = (erts_atomic_inc_read_nob(&np->slot) - 1) % 1024; + ErtsSchedulerData *esdp = erts_get_scheduler_data(); + Eterm who = THE_NON_VALUE; + ASSERT(np); + np->books[slot].what = what; + np->books[slot].term = term; + if (esdp->current_process) { + who = esdp->current_process->common.id; + } else if (esdp->current_port) { + who = esdp->current_port->common.id; + } + np->books[slot].who = who; +} +#endif + static void setup_reference_table(void) { diff --git a/erts/emulator/beam/erl_node_tables.h b/erts/emulator/beam/erl_node_tables.h index cd5d69a302..d5daf0c2df 100644 --- a/erts/emulator/beam/erl_node_tables.h +++ b/erts/emulator/beam/erl_node_tables.h @@ -176,12 +176,53 @@ struct dist_entry_ { struct dist_sequences *sequences; /* Ongoing distribution sequences */ }; +/* +#define ERL_NODE_BOOKKEEP + * Bookkeeping of ErlNode inc and dec operations to help debug refc problems. + * This is best used together with cerl -rr. Type the below into gdb: + * gdb: +set pagination off +set $i = 0 +set $node = referred_nodes[$node_ix].node +while $i < $node->slot.counter + printf "%p: ", $node->books[$i].term + etp-1 $node->books[$i].who + printf " " + p $node->books[$i].what + set $i++ +end + + * Then save that into a file called test.txt and run the below in + * an erlang shell in order to get all inc/dec that do not have a + * match. + +f(), {ok, B} = file:read_file("test.txt"). +Vs = [begin [Val, _, _, _, What] = All = string:lexemes(Ln, " "),{Val,What,All} end || Ln <- string:lexemes(B,"\n")]. +Accs = lists:foldl(fun({V,<<"ERL_NODE_INC">>,_},M) -> Val = maps:get(V,M,0), M#{ V => Val + 1 }; ({V,<<"ERL_NODE_DEC">>,_},M) -> Val = maps:get(V,M,0), M#{ V => Val - 1 } end, #{}, Vs). +lists:usort(lists:filter(fun({V,N}) -> N /= 0 end, maps:to_list(Accs))). + + * There are bound to be bugs in the the instrumentation code, but + * atleast this is a place to start when hunting refc bugs. + * + */ +#ifdef ERL_NODE_BOOKKEEP +struct erl_node_bookkeeping { + Eterm who; + Eterm term; + enum { ERL_NODE_INC, ERL_NODE_DEC } what; +}; +#endif + typedef struct erl_node_ { HashBucket hash_bucket; /* Hash bucket */ erts_refc_t refc; /* Reference count */ Eterm sysname; /* name@host atom for efficiency */ Uint32 creation; /* Creation */ DistEntry *dist_entry; /* Corresponding dist entry */ +#ifdef ERL_NODE_BOOKKEEP + struct erl_node_bookkeeping books[1024]; + erts_atomic_t slot; +#endif } ErlNode; @@ -214,7 +255,7 @@ void erts_dist_table_info(fmtfn_t, void *); void erts_set_dist_entry_not_connected(DistEntry *); void erts_set_dist_entry_pending(DistEntry *); void erts_set_dist_entry_connected(DistEntry *, Eterm, Uint); -ErlNode *erts_find_or_insert_node(Eterm, Uint32); +ErlNode *erts_find_or_insert_node(Eterm, Uint32, Eterm); void erts_schedule_delete_node(ErlNode *); void erts_set_this_node(Eterm, Uint); Uint erts_node_table_size(void); @@ -232,18 +273,38 @@ DistEntry *erts_dhandle_to_dist_entry(Eterm dhandle, Uint32* connection_id); Eterm erts_build_dhandle(Eterm **hpp, ErlOffHeap*, DistEntry*, Uint32 conn_id); Eterm erts_make_dhandle(Process *c_p, DistEntry*, Uint32 conn_id); -ERTS_GLB_INLINE void erts_deref_node_entry(ErlNode *np); +ERTS_GLB_INLINE void erts_init_node_entry(ErlNode *np, erts_aint_t val); +ERTS_GLB_INLINE erts_aint_t erts_ref_node_entry(ErlNode *np, int min_val, Eterm term); +ERTS_GLB_INLINE void erts_deref_node_entry(ErlNode *np, Eterm term); ERTS_GLB_INLINE void erts_de_rlock(DistEntry *dep); ERTS_GLB_INLINE void erts_de_runlock(DistEntry *dep); ERTS_GLB_INLINE void erts_de_rwlock(DistEntry *dep); ERTS_GLB_INLINE void erts_de_rwunlock(DistEntry *dep); +#ifdef ERL_NODE_BOOKKEEP +void erts_node_bookkeep(ErlNode *, Eterm , int); +#else +#define erts_node_bookkeep(...) +#endif #if ERTS_GLB_INLINE_INCL_FUNC_DEF ERTS_GLB_INLINE void -erts_deref_node_entry(ErlNode *np) +erts_init_node_entry(ErlNode *np, erts_aint_t val) +{ + erts_refc_init(&np->refc, val); +} + +ERTS_GLB_INLINE erts_aint_t +erts_ref_node_entry(ErlNode *np, int min_val, Eterm term) +{ + erts_node_bookkeep(np, term, ERL_NODE_INC); + return erts_refc_inctest(&np->refc, min_val); +} + +ERTS_GLB_INLINE void +erts_deref_node_entry(ErlNode *np, Eterm term) { - ASSERT(np); + erts_node_bookkeep(np, term, ERL_NODE_DEC); if (erts_refc_dectest(&np->refc, 0) == 0) erts_schedule_delete_node(np); } diff --git a/erts/emulator/beam/erl_proc_sig_queue.c b/erts/emulator/beam/erl_proc_sig_queue.c index 8113abebde..d20be0149c 100644 --- a/erts/emulator/beam/erl_proc_sig_queue.c +++ b/erts/emulator/beam/erl_proc_sig_queue.c @@ -265,7 +265,7 @@ destroy_dist_proc_demonitor(ErtsSigDistProcDemonitor *dmon) Eterm ref = dmon->ref; if (is_external(ref)) { ExternalThing *etp = external_thing_ptr(ref); - erts_deref_node_entry(etp->node); + erts_deref_node_entry(etp->node, ref); } erts_free(ERTS_ALC_T_DIST_DEMONITOR, dmon); } @@ -300,7 +300,8 @@ destroy_sig_dist_link_op(ErtsSigDistLinkOp *sdlnk) { ASSERT(is_external_pid(sdlnk->remote)); ASSERT(boxed_val(sdlnk->remote) == &sdlnk->heap[0]); - erts_deref_node_entry(((ExternalThing *) &sdlnk->heap[0])->node); + erts_deref_node_entry(((ExternalThing *) &sdlnk->heap[0])->node, + make_boxed(&sdlnk->heap[0])); erts_free(ERTS_ALC_T_SIG_DATA, sdlnk); } diff --git a/erts/emulator/beam/external.c b/erts/emulator/beam/external.c index 988ce242c4..7aa136575f 100644 --- a/erts/emulator/beam/external.c +++ b/erts/emulator/beam/external.c @@ -2480,7 +2480,7 @@ dec_atom(ErtsDistExternal *edep, byte* ep, Eterm* objp) return ep; } -static ERTS_INLINE ErlNode* dec_get_node(Eterm sysname, Uint32 creation) +static ERTS_INLINE ErlNode* dec_get_node(Eterm sysname, Uint32 creation, Eterm book) { if (sysname == INTERNAL_LOCAL_SYSNAME) /* && DFLAG_INTERNAL_TAGS */ return erts_this_node; @@ -2489,7 +2489,7 @@ static ERTS_INLINE ErlNode* dec_get_node(Eterm sysname, Uint32 creation) && (creation == erts_this_node->creation || creation == ORIG_CREATION)) return erts_this_node; - return erts_find_or_insert_node(sysname,creation); + return erts_find_or_insert_node(sysname,creation,book); } static byte* @@ -2535,7 +2535,7 @@ dec_pid(ErtsDistExternal *edep, ErtsHeapFactory* factory, byte* ep, * We are careful to create the node entry only after all * validity tests are done. */ - node = dec_get_node(sysname, cre); + node = dec_get_node(sysname, cre, make_boxed(factory->hp)); if(node == erts_this_node) { *objp = make_internal_pid(data); @@ -3529,7 +3529,7 @@ dec_term_atom_common: cre = get_int32(ep); ep += 4; } - node = dec_get_node(sysname, cre); + node = dec_get_node(sysname, cre, make_boxed(hp)); if(node == erts_this_node) { *objp = make_internal_port(num); } @@ -3609,7 +3609,7 @@ dec_term_atom_common: if (ref_words > ERTS_MAX_REF_NUMBERS) goto error; - node = dec_get_node(sysname, cre); + node = dec_get_node(sysname, cre, make_boxed(hp)); if(node == erts_this_node) { rtp = (ErtsORefThing *) hp; diff --git a/erts/emulator/beam/utils.c b/erts/emulator/beam/utils.c index 1e7b494a91..3fa09e39f2 100644 --- a/erts/emulator/beam/utils.c +++ b/erts/emulator/beam/utils.c @@ -3486,7 +3486,7 @@ store_external_or_ref_(Uint **hpp, ErlOffHeap* oh, Eterm ns) if (is_external_header(*from_hp)) { ExternalThing *etp = (ExternalThing *) from_hp; ASSERT(is_external(ns)); - erts_refc_inc(&etp->node->refc, 2); + erts_ref_node_entry(etp->node, 2, make_boxed(to_hp)); } else if (is_ordinary_ref_thing(from_hp)) return make_internal_ref(to_hp); -- cgit v1.2.3