Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys_lwmutex/lwcond: Fix race between sys_lwcond_wait vs sys_lwmutex_destroy #7826

Merged
merged 1 commit into from Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions rpcs3/Emu/Cell/PPUTranslator.cpp
Expand Up @@ -3044,8 +3044,8 @@ void PPUTranslator::DIVW(ppu_opcode_t op)
{
const auto a = GetGpr(op.ra, 32);
const auto b = GetGpr(op.rb, 32);
const auto o = m_ir->CreateOr(IsZero(b), m_ir->CreateAnd(m_ir->CreateICmpEQ(a, m_ir->getInt32(1 << 31)), IsOnes(b)));
const auto result = m_ir->CreateSDiv(a, m_ir->CreateSelect(o, m_ir->getInt32(1 << 31), b));
const auto o = m_ir->CreateOr(IsZero(b), m_ir->CreateAnd(m_ir->CreateICmpEQ(a, m_ir->getInt32(INT32_MIN)), IsOnes(b)));
const auto result = m_ir->CreateSDiv(a, m_ir->CreateSelect(o, m_ir->getInt32(INT32_MIN), b));
SetGpr(op.rd, m_ir->CreateSelect(o, m_ir->getInt32(0), result));
if (op.rc) SetCrField(0, GetUndef<bool>(), GetUndef<bool>(), GetUndef<bool>());
if (op.oe) SetOverflow(o);
Expand Down
1 change: 0 additions & 1 deletion rpcs3/Emu/Cell/lv2/sys_event_flag.cpp
Expand Up @@ -95,7 +95,6 @@ error_code sys_event_flag_wait(ppu_thread& ppu, u32 id, u64 bitptn, u32 mode, vm
sys_event_flag.trace("sys_event_flag_wait(id=0x%x, bitptn=0x%llx, mode=0x%x, result=*0x%x, timeout=0x%llx)", id, bitptn, mode, result, timeout);

// Fix function arguments for external access
// TODO: Avoid using registers
ppu.gpr[3] = -1;
ppu.gpr[4] = bitptn;
ppu.gpr[5] = mode;
Expand Down
27 changes: 25 additions & 2 deletions rpcs3/Emu/Cell/lv2/sys_lwcond.cpp
Expand Up @@ -140,7 +140,7 @@ error_code _sys_lwcond_signal(ppu_thread& ppu, u32 lwcond_id, u32 lwmutex_id, u3
{
verify(HERE), !mutex->signaled;
std::lock_guard lock(mutex->mutex);
mutex->sq.emplace_back(result);
verify(HERE), mutex->add_waiter(result);
}
else
{
Expand Down Expand Up @@ -228,7 +228,7 @@ error_code _sys_lwcond_signal_all(ppu_thread& ppu, u32 lwcond_id, u32 lwmutex_id
{
verify(HERE), !mutex->signaled;
std::lock_guard lock(mutex->mutex);
mutex->sq.emplace_back(cpu);
verify(HERE), mutex->add_waiter(cpu);
}
else
{
Expand Down Expand Up @@ -283,6 +283,23 @@ error_code _sys_lwcond_queue_wait(ppu_thread& ppu, u32 lwcond_id, u32 lwmutex_id
return;
}

// Try to increment lwmutex's lwcond's waiters count
if (!mutex->lwcond_waiters.fetch_op([](s32& val)
{
if (val == INT32_MIN)
{
return false;
}

val++;
return true;
}).second)
{
// Failed - lwmutex was detroyed and all waiters have quit
mutex.reset();
return;
}

std::lock_guard lock(cond.mutex);

// Add a waiter
Expand Down Expand Up @@ -343,6 +360,12 @@ error_code _sys_lwcond_queue_wait(ppu_thread& ppu, u32 lwcond_id, u32 lwmutex_id
}
}

if (--mutex->lwcond_waiters == INT32_MIN)
{
// Notify the thread destroying lwmutex on last waiter
mutex->lwcond_waiters.notify_all();
}

// Return cause
return not_an_error(ppu.gpr[3]);
}
59 changes: 46 additions & 13 deletions rpcs3/Emu/Cell/lv2/sys_lwmutex.cpp
Expand Up @@ -40,26 +40,54 @@ error_code _sys_lwmutex_destroy(ppu_thread& ppu, u32 lwmutex_id)

sys_lwmutex.warning("_sys_lwmutex_destroy(lwmutex_id=0x%x)", lwmutex_id);

const auto mutex = idm::withdraw<lv2_obj, lv2_lwmutex>(lwmutex_id, [&](lv2_lwmutex& mutex) -> CellError
auto mutex = idm::get<lv2_obj, lv2_lwmutex>(lwmutex_id);

if (!mutex)
{
std::lock_guard lock(mutex.mutex);
return CELL_ESRCH;
}

if (!mutex.sq.empty())
while (true)
{
if (std::scoped_lock lock(mutex->mutex); mutex->sq.empty())
{
// Set "destroyed" bit
if (mutex->lwcond_waiters.fetch_or(INT32_MIN) & 0x7fff'ffff)
{
// Deschedule if waiters were found
lv2_obj::sleep(ppu);
}
}
else
{
return CELL_EBUSY;
}

return {};
});
// Wait for all lwcond waiters to quit
if (const s32 old = mutex->lwcond_waiters; old & 0x7fff'ffff)
{
if (old > 0)
{
// Sleep queue is no longer empty
// Was set to positive value to announce it
continue;
}

if (!mutex)
{
return CELL_ESRCH;
mutex->lwcond_waiters.wait(old);

if (ppu.is_stopped())
{
return 0;
}
}

break;
}

if (mutex.ret)
if (!idm::remove_verify<lv2_obj, lv2_lwmutex>(lwmutex_id, std::move(mutex)))
{
return mutex.ret;
// Other thread has destroyed the lwmutex earlier
return CELL_ESRCH;
}

return CELL_OK;
Expand Down Expand Up @@ -95,15 +123,20 @@ error_code _sys_lwmutex_lock(ppu_thread& ppu, u32 lwmutex_id, u64 timeout)

if (old)
{
if (old == (1 << 31))
if (old == INT32_MIN)
{
ppu.gpr[3] = CELL_EBUSY;
}

return true;
}

mutex.sq.emplace_back(&ppu);
if (!mutex.add_waiter(&ppu))
{
ppu.gpr[3] = CELL_ESRCH;
return true;
}

mutex.sleep(ppu, timeout);
return false;
});
Expand Down Expand Up @@ -229,7 +262,7 @@ error_code _sys_lwmutex_unlock2(ppu_thread& ppu, u32 lwmutex_id)
return;
}

mutex.signaled |= 1 << 31;
mutex.signaled |= INT32_MIN;
});

if (!mutex)
Expand Down
33 changes: 33 additions & 0 deletions rpcs3/Emu/Cell/lv2/sys_lwmutex.h
Expand Up @@ -62,13 +62,46 @@ struct lv2_lwmutex final : lv2_obj
shared_mutex mutex;
atomic_t<s32> signaled{0};
std::deque<cpu_thread*> sq;
atomic_t<s32> lwcond_waiters{0};

lv2_lwmutex(u32 protocol, vm::ptr<sys_lwmutex_t> control, u64 name)
: protocol(protocol)
, control(control)
, name(std::bit_cast<be_t<u64>>(name))
{
}

// Try to add a waiter
bool add_waiter(cpu_thread* cpu)
{
if (const auto old = lwcond_waiters.fetch_op([](s32& val)
{
if (val + 0u <= INT32_MIN + 0u)
{
// Value was either positive or INT32_MIN
return false;
}

// lwmutex was set to be destroyed, but there are lwcond waiters
// Turn off the "destroying" bit as we are adding an lwmutex waiter
val &= 0x7fff'ffff;
return true;
}).first; old != INT32_MIN)
{
sq.emplace_back(cpu);

if (old < 0)
{
// Notify lwmutex destroyer (may cause EBUSY to be returned for it)
lwcond_waiters.notify_all();
}

return true;
}

// Failed - lwmutex was set to be destroyed and all lwcond waiters quit
return false;
}
};

// Aux
Expand Down
2 changes: 1 addition & 1 deletion rpcs3/Emu/RSX/RSXThread.cpp
Expand Up @@ -1883,7 +1883,7 @@ namespace rsx
const s32 default_frequency_mask = (1 << 8);
const s32 swap_storage_mask = (1 << 29);
const s32 volatile_storage_mask = (1 << 30);
const s32 modulo_op_frequency_mask = (1 << 31);
const s32 modulo_op_frequency_mask = (INT32_MIN);

const u32 modulo_mask = rsx::method_registers.frequency_divider_operation_mask();
const auto max_index = (first_vertex + vertex_count) - 1;
Expand Down
2 changes: 1 addition & 1 deletion rpcs3/Emu/RSX/gcm_enums.h
Expand Up @@ -600,7 +600,7 @@ enum
CELL_GCM_TEXTURE_CYLINDRICAL_WRAP_ENABLE_TEX7_U = 1 << 28,
CELL_GCM_TEXTURE_CYLINDRICAL_WRAP_ENABLE_TEX7_V = 1 << 29,
CELL_GCM_TEXTURE_CYLINDRICAL_WRAP_ENABLE_TEX7_P = 1 << 30,
CELL_GCM_TEXTURE_CYLINDRICAL_WRAP_ENABLE_TEX7_Q = 1 << 31,
CELL_GCM_TEXTURE_CYLINDRICAL_WRAP_ENABLE_TEX7_Q = 1u << 31,

CELL_GCM_COLOR_MASK_B = 1 << 0,
CELL_GCM_COLOR_MASK_G = 1 << 8,
Expand Down