aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorErlang/OTP <[email protected]>2010-01-30 08:48:16 +0000
committerErlang/OTP <[email protected]>2010-01-30 08:48:16 +0000
commit1cfb7ea778a90a234ae811666a594057198920b4 (patch)
tree08068f27b6dcc4d5c44f5e6b547016aa6a571345
parentcbc470bc904693618cc33ac16cc577401906b2f0 (diff)
parent24146d322cd165c995d3a698e888cdb6f7a10741 (diff)
downloadotp-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.)
-rw-r--r--erts/emulator/beam/erl_lock_check.c13
-rw-r--r--erts/emulator/hipe/hipe_bif0.c123
-rw-r--r--lib/kernel/src/hipe_unified_loader.erl32
3 files changed, 134 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);
}
diff --git a/lib/kernel/src/hipe_unified_loader.erl b/lib/kernel/src/hipe_unified_loader.erl
index 7e26d57ced..42eab67478 100644
--- a/lib/kernel/src/hipe_unified_loader.erl
+++ b/lib/kernel/src/hipe_unified_loader.erl
@@ -96,6 +96,14 @@ load_hipe_modules() ->
%% code:load_file/1) and the atom `no_native' on failure.
load_native_code(Mod, Bin) when is_atom(Mod), is_binary(Bin) ->
+ erlang:system_flag(multi_scheduling, block),
+ try
+ load_native_code_nosmp(Mod, Bin)
+ after
+ erlang:system_flag(multi_scheduling, unblock)
+ end.
+
+load_native_code_nosmp(Mod, Bin) ->
Architecture = erlang:system_info(hipe_architecture),
try chunk_name(Architecture) of
ChunkTag ->
@@ -120,6 +128,14 @@ load_native_code(Mod, Bin) when is_atom(Mod), is_binary(Bin) ->
-spec post_beam_load(atom()) -> 'ok'.
post_beam_load(Mod) when is_atom(Mod) ->
+ erlang:system_flag(multi_scheduling, block),
+ try
+ post_beam_load_nosmp(Mod)
+ after
+ erlang:system_flag(multi_scheduling, unblock)
+ end.
+
+post_beam_load_nosmp(Mod) ->
Architecture = erlang:system_info(hipe_architecture),
try chunk_name(Architecture) of _ChunkTag -> patch_to_emu(Mod)
catch _:_ -> ok
@@ -141,6 +157,14 @@ version_check(Version, Mod) when is_atom(Mod) ->
-spec load_module(Mod, binary(), _) -> 'bad_crc' | {'module',Mod}
when is_subtype(Mod,atom()).
load_module(Mod, Bin, Beam) ->
+ erlang:system_flag(multi_scheduling, block),
+ try
+ load_module_nosmp(Mod, Bin, Beam)
+ after
+ erlang:system_flag(multi_scheduling, unblock)
+ end.
+
+load_module_nosmp(Mod, Bin, Beam) ->
load_module(Mod, Bin, Beam, []).
load_module(Mod, Bin, Beam, OldReferencesToPatch) ->
@@ -154,6 +178,14 @@ load_module(Mod, Bin, Beam, OldReferencesToPatch) ->
-spec load(Mod, binary()) -> 'bad_crc' | {'module',Mod}
when is_subtype(Mod,atom()).
load(Mod, Bin) ->
+ erlang:system_flag(multi_scheduling, block),
+ try
+ load_nosmp(Mod, Bin)
+ after
+ erlang:system_flag(multi_scheduling, unblock)
+ end.
+
+load_nosmp(Mod, Bin) ->
?debug_msg("********* Loading funs in module ~w *********\n",[Mod]),
%% Loading just some functions in a module; patch closures separately.
put(hipe_patch_closures, true),