Lock FenceCycle inbetween waiting on chained cycles and checking signalled

Avoids one race where we would end up hogging all the locks of chained cycles and ourself when waiting for submission of previous cycles and prevent any forward progress due to another thread locking one of the chained cycles.
This commit is contained in:
Billy Laws 2022-10-09 13:04:24 +01:00
parent a015fe753d
commit 0f394d516b

View File

@ -62,6 +62,13 @@ namespace skyline::gpu {
* @brief Waits for submission of the command buffer associated with this cycle to the GPU
*/
void WaitSubmit() {
if (signalled.test(std::memory_order_consume))
return;
chainedCycles.Iterate([&](const auto &cycle) {
cycle->WaitSubmit();
});
std::unique_lock lock{mutex};
submitCondition.wait(lock, [this] { return submitted; });
}
@ -71,8 +78,6 @@ namespace skyline::gpu {
* @param shouldDestroy If true, the dependencies of this cycle will be destroyed after the fence is signalled
*/
void Wait(bool shouldDestroy = false) {
std::unique_lock lock{mutex};
if (signalled.test(std::memory_order_consume)) {
if (shouldDestroy)
DestroyDependencies();
@ -83,8 +88,15 @@ namespace skyline::gpu {
cycle->Wait(shouldDestroy);
});
std::unique_lock lock{mutex};
submitCondition.wait(lock, [&] { return submitted; });
if (signalled.test(std::memory_order_consume)) {
if (shouldDestroy)
DestroyDependencies();
return;
}
vk::Result waitResult;
while ((waitResult = (*device).waitForFences(1, &fence, false, std::numeric_limits<u64>::max(), *device.getDispatcher())) != vk::Result::eSuccess) {
if (waitResult == vk::Result::eTimeout)
@ -109,10 +121,6 @@ namespace skyline::gpu {
* @return If the wait was successful or timed out
*/
bool Wait(i64 timeoutNs, bool shouldDestroy = false) {
std::unique_lock lock{mutex, std::defer_lock};
if (!lock.try_lock_for(std::chrono::nanoseconds{timeoutNs}))
return false;
if (signalled.test(std::memory_order_consume)) {
if (shouldDestroy)
DestroyDependencies();
@ -128,6 +136,10 @@ namespace skyline::gpu {
}))
return false;
std::unique_lock lock{mutex, std::defer_lock};
if (!lock.try_lock_for(std::chrono::nanoseconds{timeoutNs}))
return false;
if (!submitCondition.wait_for(lock, std::chrono::nanoseconds(timeoutNs), [&] { return submitted; }))
return false;
@ -165,10 +177,6 @@ namespace skyline::gpu {
* @return If the fence is signalled currently or not
*/
bool Poll(bool quick = true, bool shouldDestroy = false) {
std::unique_lock lock{mutex, std::try_to_lock};
if (!lock)
return false;
if (signalled.test(std::memory_order_consume)) {
if (shouldDestroy)
DestroyDependencies();
@ -181,6 +189,10 @@ namespace skyline::gpu {
if (!chainedCycles.AllOf([=](auto &cycle) { return cycle->Poll(quick, shouldDestroy); }))
return false;
std::unique_lock lock{mutex, std::try_to_lock};
if (!lock)
return false;
if (!submitted)
return false;
@ -217,7 +229,7 @@ namespace skyline::gpu {
* @param cycle The cycle to chain to this one, this is nullable and this function will be a no-op if this is nullptr
*/
void ChainCycle(const std::shared_ptr<FenceCycle> &cycle) {
if (cycle && !signalled.test(std::memory_order_consume) && cycle.get() != this && !cycle->Poll())
if (cycle && !signalled.test(std::memory_order_consume) && cycle.get() != this)
chainedCycles.Append(cycle); // If the cycle isn't the current cycle or already signalled, we need to chain it
}