Skip to content

Commit

Permalink
Fix atomics usage related to actor muting for ARM (#3552)
Browse files Browse the repository at this point in the history
After a review of the memory ordering with atomics related to backpressure
and actor muting, Sylvan's statement was "this is broken on ARM".

Here's the fixes.
  • Loading branch information
SeanTAllen authored May 9, 2020
1 parent a298423 commit 4070679
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
20 changes: 10 additions & 10 deletions src/libponyrt/actor/actor.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ static bool maybe_mute(pony_actor_t* actor)
// a behavior.
// 2. We should bail out from running the actor and return false so that
// it won't be rescheduled.
if(atomic_load_explicit(&actor->muted, memory_order_relaxed) > 0)
if(atomic_load_explicit(&actor->muted, memory_order_acquire) > 0)
{
ponyint_mute_actor(actor);
return true;
Expand Down Expand Up @@ -872,7 +872,7 @@ PONY_API void pony_schedule(pony_ctx_t* ctx, pony_actor_t* actor)
PONY_API void pony_unschedule(pony_ctx_t* ctx, pony_actor_t* actor)
{
(void)ctx;

if(has_flag(actor, FLAG_BLOCKED_SENT))
{
// send unblock if we've sent a block
Expand Down Expand Up @@ -967,28 +967,28 @@ bool ponyint_triggers_muting(pony_actor_t* actor)
// We have far more relaxed usage of atomics in `ponyint_mute_actor` given the
// far more relaxed rule #2.
//
// An actor's `is_muted` field is effectly a `bool` value. However, by using a
// `uint8_t`, we use the same amount of space that we would for a boolean but
// can use more efficient atomic operations. Given how often these methods are
// called (at least once per message send), efficiency is of primary
// importance.
// An actor's `is_muted` field is effectively a `bool` value. However, by
// using a `uint8_t`, we use the same amount of space that we would for a
// boolean but can use more efficient atomic operations. Given how often
// these methods are called (at least once per message send), efficiency is
// of primary importance.

bool ponyint_is_muted(pony_actor_t* actor)
{
return (atomic_load_explicit(&actor->is_muted, memory_order_relaxed) > 0);
return (atomic_load_explicit(&actor->is_muted, memory_order_acquire) > 0);
}

void ponyint_mute_actor(pony_actor_t* actor)
{
uint8_t is_muted = atomic_fetch_add_explicit(&actor->is_muted, 1, memory_order_relaxed);
uint8_t is_muted = atomic_fetch_add_explicit(&actor->is_muted, 1, memory_order_acq_rel);
pony_assert(is_muted == 0);
DTRACE1(ACTOR_MUTED, (uintptr_t)actor);
(void)is_muted;
}

void ponyint_unmute_actor(pony_actor_t* actor)
{
uint8_t is_muted = atomic_fetch_sub_explicit(&actor->is_muted, 1, memory_order_relaxed);
uint8_t is_muted = atomic_fetch_sub_explicit(&actor->is_muted, 1, memory_order_acq_rel);
pony_assert(is_muted == 1);
DTRACE1(ACTOR_UNMUTED, (uintptr_t)actor);
(void)is_muted;
Expand Down
4 changes: 2 additions & 2 deletions src/libponyrt/sched/scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,7 @@ void ponyint_sched_mute(pony_ctx_t* ctx, pony_actor_t* sender, pony_actor_t* rec
int64_t old_mset_alloc_size = ponyint_muteset_alloc_size(&mref->value);
#endif
ponyint_muteset_putindex(&mref->value, sender, index2);
atomic_fetch_add_explicit(&sender->muted, 1, memory_order_relaxed);
atomic_fetch_add_explicit(&sender->muted, 1, memory_order_acq_rel);
#ifdef USE_MEMTRACK
int64_t new_mset_mem_size = ponyint_muteset_mem_size(&mref->value);
int64_t new_mset_alloc_size = ponyint_muteset_alloc_size(&mref->value);
Expand Down Expand Up @@ -1454,7 +1454,7 @@ bool ponyint_sched_unmute_senders(pony_ctx_t* ctx, pony_actor_t* actor)
{
// This is safe because an actor can only ever be in a single scheduler's
// mutemap
size_t muted_count = atomic_fetch_sub_explicit(&muted->muted, 1, memory_order_relaxed);
size_t muted_count = atomic_fetch_sub_explicit(&muted->muted, 1, memory_order_acq_rel);
pony_assert(muted_count > 0);

// If muted_count used to be 1 before we decremented it; then the actor
Expand Down

0 comments on commit 4070679

Please sign in to comment.