From 031fd45d17d4887fdd437ae73608e55e76660781 Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Fri, 8 Apr 2011 14:50:26 +0200 Subject: Avoid scheduling of processes being garbage collected by others --- erts/emulator/beam/beam_bif_load.c | 12 +++++-- erts/emulator/beam/bif.c | 24 +++++++++---- erts/emulator/beam/erl_process.c | 66 +++++++++++++++++++++++++++--------- erts/emulator/beam/erl_process.h | 4 +++ erts/emulator/test/process_SUITE.erl | 40 ++++++++++++++++++++-- 5 files changed, 119 insertions(+), 27 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/beam_bif_load.c b/erts/emulator/beam/beam_bif_load.c index 6ae9736141..b6893ff547 100644 --- a/erts/emulator/beam/beam_bif_load.c +++ b/erts/emulator/beam/beam_bif_load.c @@ -175,8 +175,12 @@ check_process_code_2(BIF_ALIST_2) Eterm res; if (internal_pid_index(BIF_ARG_1) >= erts_max_processes) goto error; - rp = erts_pid2proc_not_running(BIF_P, ERTS_PROC_LOCK_MAIN, - BIF_ARG_1, ERTS_PROC_LOCK_MAIN); +#ifdef ERTS_SMP + rp = erts_pid2proc_suspend(BIF_P, ERTS_PROC_LOCK_MAIN, + BIF_ARG_1, ERTS_PROC_LOCK_MAIN); +#else + rp = erts_pid2proc(BIF_P, 0, BIF_ARG_1, 0); +#endif if (!rp) { BIF_RET(am_false); } @@ -187,8 +191,10 @@ check_process_code_2(BIF_ALIST_2) modp = erts_get_module(BIF_ARG_2); res = check_process_code(rp, modp); #ifdef ERTS_SMP - if (BIF_P != rp) + if (BIF_P != rp) { + erts_resume(rp, ERTS_PROC_LOCK_MAIN); erts_smp_proc_unlock(rp, ERTS_PROC_LOCK_MAIN); + } #endif BIF_RET(res); } diff --git a/erts/emulator/beam/bif.c b/erts/emulator/beam/bif.c index f01580eb2b..53a18cb2e9 100644 --- a/erts/emulator/beam/bif.c +++ b/erts/emulator/beam/bif.c @@ -3217,20 +3217,32 @@ BIF_RETTYPE garbage_collect_1(BIF_ALIST_1) BIF_ERROR(BIF_P, BADARG); } - rp = erts_pid2proc_not_running(BIF_P, ERTS_PROC_LOCK_MAIN, + if (BIF_P->id == BIF_ARG_1) + rp = BIF_P; + else { +#ifdef ERTS_SMP + rp = erts_pid2proc_suspend(BIF_P, ERTS_PROC_LOCK_MAIN, BIF_ARG_1, ERTS_PROC_LOCK_MAIN); - if (!rp) - BIF_RET(am_false); - if (rp == ERTS_PROC_LOCK_BUSY) - ERTS_BIF_YIELD1(bif_export[BIF_garbage_collect_1], BIF_P, BIF_ARG_1); + if (rp == ERTS_PROC_LOCK_BUSY) + ERTS_BIF_YIELD1(bif_export[BIF_garbage_collect_1], BIF_P, BIF_ARG_1); +#else + rp = erts_pid2proc(BIF_P, 0, BIF_ARG_1, 0); +#endif + if (!rp) + BIF_RET(am_false); + } /* The GC cost is taken for the process executing this BIF. */ FLAGS(rp) |= F_NEED_FULLSWEEP; reds = erts_garbage_collect(rp, 0, rp->arg_reg, rp->arity); - if (BIF_P != rp) +#ifdef ERTS_SMP + if (BIF_P != rp) { + erts_resume(rp, ERTS_PROC_LOCK_MAIN); erts_smp_proc_unlock(rp, ERTS_PROC_LOCK_MAIN); + } +#endif BIF_RET2(am_true, reds); } diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c index 21625921d5..8c3d21b98f 100644 --- a/erts/emulator/beam/erl_process.c +++ b/erts/emulator/beam/erl_process.c @@ -3889,21 +3889,9 @@ handle_pend_sync_suspend(Process *suspendee, } } -/* - * Like erts_pid2proc() but: - * - * * At least ERTS_PROC_LOCK_MAIN have to be held on c_p. - * * At least ERTS_PROC_LOCK_MAIN have to be taken on pid. - * * It also waits for proc to be in a state != running and garbing. - * * If ERTS_PROC_LOCK_BUSY is returned, the calling process has to - * yield (ERTS_BIF_YIELD[0-3]()). c_p might in this case have been - * suspended. - */ - - -Process * -erts_pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, - Eterm pid, ErtsProcLocks pid_locks) +static Process * +pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, + Eterm pid, ErtsProcLocks pid_locks, int suspend) { Process *rp; int unlock_c_p_status; @@ -3930,7 +3918,7 @@ erts_pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, c_p->suspendee = NIL; ASSERT(c_p->flags & F_P2PNR_RESCHED); c_p->flags &= ~F_P2PNR_RESCHED; - if (rp) + if (!suspend && rp) resume_process(rp); } else { @@ -3994,6 +3982,8 @@ erts_pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, } /* rp is not running and we got the locks we want... */ + if (suspend) + suspend_process(rp_rq, rp); } erts_smp_runqs_unlock(cp_rq, rp_rq); } @@ -4006,6 +3996,35 @@ erts_pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, return rp; } + +/* + * Like erts_pid2proc() but: + * + * * At least ERTS_PROC_LOCK_MAIN have to be held on c_p. + * * At least ERTS_PROC_LOCK_MAIN have to be taken on pid. + * * It also waits for proc to be in a state != running and garbing. + * * If ERTS_PROC_LOCK_BUSY is returned, the calling process has to + * yield (ERTS_BIF_YIELD[0-3]()). c_p might in this case have been + * suspended. + */ +Process * +erts_pid2proc_not_running(Process *c_p, ErtsProcLocks c_p_locks, + Eterm pid, ErtsProcLocks pid_locks) +{ + return pid2proc_not_running(c_p, c_p_locks, pid, pid_locks, 0); +} + +/* + * Like erts_pid2proc_not_running(), but hands over the process + * in a suspended state unless (c_p is looked up). + */ +Process * +erts_pid2proc_suspend(Process *c_p, ErtsProcLocks c_p_locks, + Eterm pid, ErtsProcLocks pid_locks) +{ + return pid2proc_not_running(c_p, c_p_locks, pid, pid_locks, 1); +} + /* * erts_pid2proc_nropt() is normally the same as * erts_pid2proc_not_running(). However it is only @@ -4119,6 +4138,21 @@ handle_pend_bif_async_suspend(Process *suspendee, } } +#else + +/* + * Non-smp version of erts_pid2proc_suspend(). + */ +Process * +erts_pid2proc_suspend(Process *c_p, ErtsProcLocks c_p_locks, + Eterm pid, ErtsProcLocks pid_locks) +{ + Process *rp = erts_pid2proc(c_p, c_p_locks, pid, pid_locks); + if (rp) + erts_suspend(rp, pid_locks, NULL); + return rp; +} + #endif /* ERTS_SMP */ /* diff --git a/erts/emulator/beam/erl_process.h b/erts/emulator/beam/erl_process.h index 334ae5573f..296acc7367 100644 --- a/erts/emulator/beam/erl_process.h +++ b/erts/emulator/beam/erl_process.h @@ -1535,6 +1535,10 @@ erts_get_atom_cache_map(Process *c_p) } #endif +Process *erts_pid2proc_suspend(Process *, + ErtsProcLocks, + Eterm, + ErtsProcLocks); #ifdef ERTS_SMP Process *erts_pid2proc_not_running(Process *, diff --git a/erts/emulator/test/process_SUITE.erl b/erts/emulator/test/process_SUITE.erl index 36bae908aa..f68e712268 100644 --- a/erts/emulator/test/process_SUITE.erl +++ b/erts/emulator/test/process_SUITE.erl @@ -50,7 +50,8 @@ processes_last_call_trap/1, processes_gc_trap/1, processes_term_proc_list/1, otp_7738_waiting/1, otp_7738_suspended/1, - otp_7738_resume/1]). + otp_7738_resume/1, + garb_other_running/1]). -export([prio_server/2, prio_client/2]). -export([init_per_testcase/2, end_per_testcase/2]). @@ -72,7 +73,7 @@ all() -> bad_register, garbage_collect, process_info_messages, process_flag_badarg, process_flag_heap_size, spawn_opt_heap_size, otp_6237, {group, processes_bif}, - {group, otp_7738}]. + {group, otp_7738}, garb_other_running]. groups() -> [{t_exit_2, [], @@ -2116,6 +2117,41 @@ otp_7738_test(Type) -> end, ?line ok. +gor(Reds, Stop) -> + receive + {From, reds} -> + From ! {reds, Reds, self()}, + gor(Reds+1, Stop); + {From, Stop} -> + From ! {stopped, Stop, Reds, self()} + after 0 -> + gor(Reds+1, Stop) + end. + +garb_other_running(Config) when is_list(Config) -> + ?line Stop = make_ref(), + ?line {Pid, Mon} = spawn_monitor(fun () -> gor(0, Stop) end), + ?line Reds = lists:foldl(fun (_, OldReds) -> + ?line erlang:garbage_collect(Pid), + ?line receive after 1 -> ok end, + ?line Pid ! {self(), reds}, + ?line receive + {reds, NewReds, Pid} -> + ?line true = (NewReds > OldReds), + ?line NewReds + end + end, + 0, + lists:seq(1, 10000)), + ?line receive after 1 -> ok end, + ?line Pid ! {self(), Stop}, + ?line receive + {stopped, Stop, StopReds, Pid} -> + ?line true = (StopReds > Reds) + end, + ?line receive {'DOWN', Mon, process, Pid, normal} -> ok end, + ?line ok. + %% Internal functions wait_until(Fun) -> -- cgit v1.2.1 From aeda9e6b3b28849a27b0f4f5386d000e40a198d0 Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Wed, 11 May 2011 14:38:24 +0200 Subject: Fix bad assertions --- erts/emulator/beam/erl_process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'erts') diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c index 8c3d21b98f..b286ec0269 100644 --- a/erts/emulator/beam/erl_process.c +++ b/erts/emulator/beam/erl_process.c @@ -4686,7 +4686,7 @@ internal_add_to_runq(ErtsRunQueue *runq, Process *p) if (p->status_flags & ERTS_PROC_SFLG_INRUNQ) return NULL; else if (p->runq_flags & ERTS_PROC_RUNQ_FLG_RUNNING) { - ASSERT(p->rcount == 0); + ASSERT(ERTS_PROC_IS_EXITING(p) || p->rcount == 0); ERTS_DBG_CHK_PROCS_RUNQ_NOPROC(runq, p); p->status_flags |= ERTS_PROC_SFLG_PENDADD2SCHEDQ; return NULL; @@ -4697,7 +4697,7 @@ internal_add_to_runq(ErtsRunQueue *runq, Process *p) ERTS_DBG_CHK_PROCS_RUNQ_NOPROC(runq, p); #ifndef ERTS_SMP /* Never schedule a suspended process (ok in smp case) */ - ASSERT(p->rcount == 0); + ASSERT(ERTS_PROC_IS_EXITING(p) || p->rcount == 0); add_runq = runq; #else ASSERT(!p->bound_runq || p->bound_runq == p->run_queue); -- cgit v1.2.1