diff options
author | Erlang/OTP <otp@erlang.org> | 2010-01-30 08:48:16 +0000 |
---|---|---|
committer | Erlang/OTP <otp@erlang.org> | 2010-01-30 08:48:16 +0000 |
commit | 1cfb7ea778a90a234ae811666a594057198920b4 (patch) | |
tree | 08068f27b6dcc4d5c44f5e6b547016aa6a571345 /erts/emulator | |
parent | cbc470bc904693618cc33ac16cc577401906b2f0 (diff) | |
parent | 24146d322cd165c995d3a698e888cdb6f7a10741 (diff) | |
download | otp-1cfb7ea778a90a234ae811666a594057198920b4.tar.gz otp-1cfb7ea778a90a234ae811666a594057198920b4.tar.bz2 otp-1cfb7ea778a90a234ae811666a594057198920b4.zip |
Merge branch 'mp/hipe-smp-fixes' into ccase/r13b04_dev
* mp/hipe-smp-fixes:
work around hipe_mfa_info_table lock omission
fix hipe loader SMP non-atomicity error
OTP-8397 The loading of native code was not properly atomic in the SMP
emulator, which could cause crashes. Also a per-MFA information
table for the native code has now been protected with a lock
since it turns that it could be accessed concurrently in the SMP
emulator. (Thanks to Mikael Pettersson.)
Diffstat (limited to 'erts/emulator')
-rw-r--r-- | erts/emulator/beam/erl_lock_check.c | 13 | ||||
-rw-r--r-- | erts/emulator/hipe/hipe_bif0.c | 123 |
2 files changed, 102 insertions, 34 deletions
diff --git a/erts/emulator/beam/erl_lock_check.c b/erts/emulator/beam/erl_lock_check.c index 25f1d420d1..2f2924ae76 100644 --- a/erts/emulator/beam/erl_lock_check.c +++ b/erts/emulator/beam/erl_lock_check.c @@ -1,19 +1,19 @@ /* * %CopyrightBegin% - * - * Copyright Ericsson AB 2005-2009. All Rights Reserved. - * + * + * Copyright Ericsson AB 2005-2010. All Rights Reserved. + * * The contents of this file are subject to the Erlang Public License, * Version 1.1, (the "License"); you may not use this file except in * compliance with the License. You should have received a copy of the * Erlang Public License along with this software. If not, it can be * retrieved online at http://www.erlang.org/. - * + * * Software distributed under the License is distributed on an "AS IS" * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See * the License for the specific language governing rights and limitations * under the License. - * + * * %CopyrightEnd% */ @@ -75,6 +75,9 @@ static erts_lc_lock_order_t erts_lock_order[] = { * the lock name)" */ #ifdef ERTS_SMP +#ifdef HIPE + { "hipe_mfait_lock", NULL }, +#endif { "driver_lock", "driver_name" }, { "port_lock", "port_id" }, #endif diff --git a/erts/emulator/hipe/hipe_bif0.c b/erts/emulator/hipe/hipe_bif0.c index 032bf2e896..5291374e25 100644 --- a/erts/emulator/hipe/hipe_bif0.c +++ b/erts/emulator/hipe/hipe_bif0.c @@ -1,19 +1,19 @@ /* * %CopyrightBegin% - * - * Copyright Ericsson AB 2001-2009. All Rights Reserved. - * + * + * Copyright Ericsson AB 2001-2010. All Rights Reserved. + * * The contents of this file are subject to the Erlang Public License, * Version 1.1, (the "License"); you may not use this file except in * compliance with the License. You should have received a copy of the * Erlang Public License along with this software. If not, it can be * retrieved online at http://www.erlang.org/. - * + * * Software distributed under the License is distributed on an "AS IS" * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See * the License for the specific language governing rights and limitations * under the License. - * + * * %CopyrightEnd% */ /* $Id$ @@ -656,6 +656,7 @@ static void *hipe_get_emu_address(Eterm m, Eterm f, unsigned int arity, int is_r address = hipe_find_emu_address(m, f, arity); if (!address) { /* if not found, stub it via the export entry */ + /* no lock needed around erts_export_get_or_make_stub() */ Export *export_entry = erts_export_get_or_make_stub(m, f, arity); address = export_entry->address; } @@ -1188,8 +1189,31 @@ static struct { unsigned int mask; /* INV: mask == (1 << log2size)-1 */ unsigned int used; struct hipe_mfa_info **bucket; + /* + * The mfa info table is normally updated by the loader, + * which runs in non-concurrent mode. Unfortunately runtime + * apply operations (get_na_nofail) update the table if + * they create a new stub for the mfa, which forces locking. + * XXX: Redesign apply et al to avoid those updates. + */ + erts_smp_mtx_t lock; } hipe_mfa_info_table; +static inline void hipe_mfa_info_table_init_lock(void) +{ + erts_smp_mtx_init(&hipe_mfa_info_table.lock, "hipe_mfait_lock"); +} + +static inline void hipe_mfa_info_table_lock(void) +{ + erts_smp_mtx_lock(&hipe_mfa_info_table.lock); +} + +static inline void hipe_mfa_info_table_unlock(void) +{ + erts_smp_mtx_unlock(&hipe_mfa_info_table.lock); +} + #define HIPE_MFA_HASH(M,F,A) ((M) * (F) + (A)) static struct hipe_mfa_info **hipe_mfa_info_table_alloc_bucket(unsigned int size) @@ -1258,9 +1282,11 @@ void hipe_mfa_info_table_init(void) hipe_mfa_info_table.mask = size - 1; hipe_mfa_info_table.used = 0; hipe_mfa_info_table.bucket = hipe_mfa_info_table_alloc_bucket(size); + + hipe_mfa_info_table_init_lock(); } -static inline struct hipe_mfa_info *hipe_mfa_info_table_get(Eterm m, Eterm f, unsigned int arity) +static inline struct hipe_mfa_info *hipe_mfa_info_table_get_locked(Eterm m, Eterm f, unsigned int arity) { unsigned long h; unsigned int i; @@ -1286,7 +1312,7 @@ void *hipe_mfa_find_na(Eterm m, Eterm f, unsigned int arity) } #endif -static struct hipe_mfa_info *hipe_mfa_info_table_put(Eterm m, Eterm f, unsigned int arity) +static struct hipe_mfa_info *hipe_mfa_info_table_put_locked(Eterm m, Eterm f, unsigned int arity) { unsigned long h; unsigned int i; @@ -1313,7 +1339,10 @@ static struct hipe_mfa_info *hipe_mfa_info_table_put(Eterm m, Eterm f, unsigned static void hipe_mfa_set_na(Eterm m, Eterm f, unsigned int arity, void *address, int is_exported) { - struct hipe_mfa_info *p = hipe_mfa_info_table_put(m, f, arity); + struct hipe_mfa_info *p; + + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_put_locked(m, f, arity); #ifdef DEBUG_LINKER printf("%s: ", __FUNCTION__); print_mfa(m, f, arity); @@ -1322,19 +1351,30 @@ static void hipe_mfa_set_na(Eterm m, Eterm f, unsigned int arity, void *address, p->local_address = address; if (is_exported) p->remote_address = address; + hipe_mfa_info_table_unlock(); } #if defined(__powerpc__) || defined(__ppc__) || defined(__powerpc64__) || defined(__arm__) void *hipe_mfa_get_trampoline(Eterm m, Eterm f, unsigned int arity) { - struct hipe_mfa_info *p = hipe_mfa_info_table_put(m, f, arity); - return p->trampoline; + struct hipe_mfa_info *p; + void *trampoline; + + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_put_locked(m, f, arity); + trampoline = p->trampoline; + hipe_mfa_info_table_unlock(); + return trampoline; } void hipe_mfa_set_trampoline(Eterm m, Eterm f, unsigned int arity, void *trampoline) { - struct hipe_mfa_info *p = hipe_mfa_info_table_put(m, f, arity); + struct hipe_mfa_info *p; + + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_put_locked(m, f, arity); p->trampoline = trampoline; + hipe_mfa_info_table_unlock(); } #endif @@ -1365,12 +1405,13 @@ BIF_RETTYPE hipe_bifs_invalidate_funinfo_native_addresses_1(BIF_ALIST_1) struct mfa mfa; struct hipe_mfa_info *p; + hipe_mfa_info_table_lock(); lst = BIF_ARG_1; while (is_list(lst)) { if (!term_to_mfa(CAR(list_val(lst)), &mfa)) - BIF_ERROR(BIF_P, BADARG); + break; lst = CDR(list_val(lst)); - p = hipe_mfa_info_table_get(mfa.mod, mfa.fun, mfa.ari); + p = hipe_mfa_info_table_get_locked(mfa.mod, mfa.fun, mfa.ari); if (p) { p->remote_address = NULL; p->local_address = NULL; @@ -1393,6 +1434,7 @@ BIF_RETTYPE hipe_bifs_invalidate_funinfo_native_addresses_1(BIF_ALIST_1) } } } + hipe_mfa_info_table_unlock(); if (is_not_nil(lst)) BIF_ERROR(BIF_P, BADARG); BIF_RET(NIL); @@ -1406,7 +1448,8 @@ void hipe_mfa_save_orig_beam_op(Eterm mod, Eterm fun, unsigned int ari, Eterm *p orig_beam_op = pc[0]; if (orig_beam_op != BeamOpCode(op_hipe_trap_call_closure) && orig_beam_op != BeamOpCode(op_hipe_trap_call)) { - p = hipe_mfa_info_table_put(mod, fun, ari); + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_put_locked(mod, fun, ari); #ifdef DEBUG_LINKER printf("%s: ", __FUNCTION__); print_mfa(mod, fun, ari); @@ -1414,6 +1457,7 @@ void hipe_mfa_save_orig_beam_op(Eterm mod, Eterm fun, unsigned int ari, Eterm *p #endif p->beam_code = pc; p->orig_beam_op = orig_beam_op; + hipe_mfa_info_table_unlock(); } else { #ifdef DEBUG_LINKER printf("%s: ", __FUNCTION__); @@ -1440,12 +1484,12 @@ static void *hipe_make_stub(Eterm m, Eterm f, unsigned int arity, int is_remote) return StubAddress; } -static void *hipe_get_na_nofail(Eterm m, Eterm f, unsigned int a, int is_remote) +static void *hipe_get_na_nofail_locked(Eterm m, Eterm f, unsigned int a, int is_remote) { struct hipe_mfa_info *p; void *address; - p = hipe_mfa_info_table_get(m, f, a); + p = hipe_mfa_info_table_get_locked(m, f, a); if (p) { /* find address, predicting for a runtime apply call */ address = p->remote_address; @@ -1459,13 +1503,23 @@ static void *hipe_get_na_nofail(Eterm m, Eterm f, unsigned int a, int is_remote) if (address) return address; } else - p = hipe_mfa_info_table_put(m, f, a); + p = hipe_mfa_info_table_put_locked(m, f, a); address = hipe_make_stub(m, f, a, is_remote); /* XXX: how to tell if a BEAM MFA is exported or not? */ p->remote_address = address; return address; } +static void *hipe_get_na_nofail(Eterm m, Eterm f, unsigned int a, int is_remote) +{ + void *p; + + hipe_mfa_info_table_lock(); + p = hipe_get_na_nofail_locked(m, f, a, is_remote); + hipe_mfa_info_table_unlock(); + return p; +} + /* used for apply/3 in hipe_mode_switch */ void *hipe_get_remote_na(Eterm m, Eterm f, unsigned int a) { @@ -1549,6 +1603,8 @@ int hipe_find_mfa_from_ra(const void *ra, Eterm *m, Eterm *f, unsigned int *a) /* Note about locking: the table is only updated from the loader, which runs with the rest of the system suspended. */ + /* XXX: alas not true; see comment at hipe_mfa_info_table.lock */ + hipe_mfa_info_table_lock(); bucket = hipe_mfa_info_table.bucket; nrbuckets = 1 << hipe_mfa_info_table.log2size; mfa = NULL; @@ -1564,12 +1620,13 @@ int hipe_find_mfa_from_ra(const void *ra, Eterm *m, Eterm *f, unsigned int *a) b = b->bucket.next; } } - if (!mfa) - return 0; - *m = mfa->m; - *f = mfa->f; - *a = mfa->a; - return 1; + if (mfa) { + *m = mfa->m; + *f = mfa->f; + *a = mfa->a; + } + hipe_mfa_info_table_unlock(); + return mfa ? 1 : 0; } /* @@ -1645,8 +1702,9 @@ BIF_RETTYPE hipe_bifs_add_ref_2(BIF_ALIST_2) default: goto badarg; } - callee_mfa = hipe_mfa_info_table_put(callee.mod, callee.fun, callee.ari); - caller_mfa = hipe_mfa_info_table_put(caller.mod, caller.fun, caller.ari); + hipe_mfa_info_table_lock(); + callee_mfa = hipe_mfa_info_table_put_locked(callee.mod, callee.fun, callee.ari); + caller_mfa = hipe_mfa_info_table_put_locked(caller.mod, caller.fun, caller.ari); refers_to = erts_alloc(ERTS_ALC_T_HIPE, sizeof(*refers_to)); refers_to->mfa = callee_mfa; @@ -1660,6 +1718,7 @@ BIF_RETTYPE hipe_bifs_add_ref_2(BIF_ALIST_2) ref->flags = flags; ref->next = callee_mfa->referred_from; callee_mfa->referred_from = ref; + hipe_mfa_info_table_unlock(); BIF_RET(NIL); @@ -1679,10 +1738,12 @@ BIF_RETTYPE hipe_bifs_mark_referred_from_1(BIF_ALIST_1) /* get_refs_from */ if (!term_to_mfa(BIF_ARG_1, &mfa)) BIF_ERROR(BIF_P, BADARG); - p = hipe_mfa_info_table_get(mfa.mod, mfa.fun, mfa.ari); + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_get_locked(mfa.mod, mfa.fun, mfa.ari); if (p) for (ref = p->referred_from; ref != NULL; ref = ref->next) ref->flags |= REF_FLAG_PENDING_REDIRECT; + hipe_mfa_info_table_unlock(); BIF_RET(NIL); } @@ -1695,7 +1756,8 @@ BIF_RETTYPE hipe_bifs_remove_refs_from_1(BIF_ALIST_1) if (!term_to_mfa(BIF_ARG_1, &mfa)) BIF_ERROR(BIF_P, BADARG); - caller_mfa = hipe_mfa_info_table_get(mfa.mod, mfa.fun, mfa.ari); + hipe_mfa_info_table_lock(); + caller_mfa = hipe_mfa_info_table_get_locked(mfa.mod, mfa.fun, mfa.ari); if (caller_mfa) { refers_to = caller_mfa->refers_to; while (refers_to) { @@ -1725,6 +1787,7 @@ BIF_RETTYPE hipe_bifs_remove_refs_from_1(BIF_ALIST_1) } caller_mfa->refers_to = NULL; } + hipe_mfa_info_table_unlock(); BIF_RET(NIL); } @@ -1742,14 +1805,15 @@ BIF_RETTYPE hipe_bifs_redirect_referred_from_1(BIF_ALIST_1) if (!term_to_mfa(BIF_ARG_1, &mfa)) BIF_ERROR(BIF_P, BADARG); - p = hipe_mfa_info_table_get(mfa.mod, mfa.fun, mfa.ari); + hipe_mfa_info_table_lock(); + p = hipe_mfa_info_table_get_locked(mfa.mod, mfa.fun, mfa.ari); if (p) { prev = &p->referred_from; ref = *prev; while (ref) { if (ref->flags & REF_FLAG_PENDING_REDIRECT) { is_remote = ref->flags & REF_FLAG_IS_REMOTE; - new_address = hipe_get_na_nofail(p->m, p->f, p->a, is_remote); + new_address = hipe_get_na_nofail_locked(p->m, p->f, p->a, is_remote); if (ref->flags & REF_FLAG_IS_LOAD_MFA) res = hipe_patch_insn(ref->address, (Uint)new_address, am_load_mfa); else @@ -1772,6 +1836,7 @@ BIF_RETTYPE hipe_bifs_redirect_referred_from_1(BIF_ALIST_1) } } } + hipe_mfa_info_table_unlock(); BIF_RET(NIL); } |