Avoid waiting on texture/buffer fence with trapMutex locked

This could cause deadlocks under certain circumstances by preventing the GPFIFO from making any progress while also waiting on it at the same time.
This commit is contained in:
Billy Laws 2022-10-09 12:55:56 +01:00
parent 3e8bd26978
commit 993ffb56f4
2 changed files with 40 additions and 9 deletions

View File

@ -23,11 +23,23 @@ namespace skyline::gpu {
return;
std::unique_lock stateLock{buffer->stateMutex};
if (buffer->AllCpuBackingWritesBlocked()) {
if (buffer->AllCpuBackingWritesBlocked() || buffer->dirtyState == DirtyState::GpuDirty) {
stateLock.unlock(); // If the lock isn't unlocked, a deadlock from threads waiting on the other lock can occur
// If this mutex would cause other callbacks to be blocked then we should block on this mutex in advance
std::scoped_lock lock{*buffer};
std::shared_ptr<FenceCycle> waitCycle{};
do {
if (waitCycle)
waitCycle->Wait();
std::scoped_lock lock{*buffer};
if (waitCycle && buffer->cycle == waitCycle) {
buffer->cycle = {};
waitCycle = {};
} else {
waitCycle = buffer->cycle;
}
} while (waitCycle);
}
}, [weakThis] {
TRACE_EVENT("gpu", "Buffer::ReadTrap");
@ -47,6 +59,9 @@ namespace skyline::gpu {
if (!lock)
return false;
if (buffer->cycle)
return false;
buffer->SynchronizeGuest(true); // We can skip trapping since the caller will do it
return true;
}, [weakThis] {
@ -69,7 +84,9 @@ namespace skyline::gpu {
if (!lock)
return false;
buffer->WaitOnFence();
if (buffer->cycle)
return false;
buffer->SynchronizeGuest(true); // We need to assume the buffer is dirty since we don't know what the guest is writing
buffer->dirtyState = DirtyState::CpuDirty;
@ -128,8 +145,6 @@ namespace skyline::gpu {
void Buffer::WaitOnFence() {
TRACE_EVENT("gpu", "Buffer::WaitOnFence");
std::scoped_lock lock{stateMutex};
if (cycle) {
cycle->Wait();
cycle = nullptr;
@ -137,8 +152,6 @@ namespace skyline::gpu {
}
bool Buffer::PollFence() {
std::scoped_lock lock{stateMutex};
if (!cycle)
return true;
@ -219,7 +232,6 @@ namespace skyline::gpu {
}
void Buffer::Read(bool isFirstUsage, const std::function<void()> &flushHostCallback, span<u8> data, vk::DeviceSize offset) {
std::scoped_lock lock{stateMutex};
if (dirtyState == DirtyState::GpuDirty)
SynchronizeGuestImmediate(isFirstUsage, flushHostCallback);

View File

@ -159,7 +159,19 @@ namespace skyline::gpu {
stateLock.unlock(); // If the lock isn't unlocked, a deadlock from threads waiting on the other lock can occur
// If this mutex would cause other callbacks to be blocked then we should block on this mutex in advance
std::scoped_lock lock{*texture};
std::shared_ptr<FenceCycle> waitCycle{};
do {
if (waitCycle)
waitCycle->Wait();
std::scoped_lock lock{*texture};
if (waitCycle && texture->cycle == waitCycle) {
texture->cycle = {};
waitCycle = {};
} else {
waitCycle = texture->cycle;
}
} while (waitCycle);
}
}, [weakThis] {
TRACE_EVENT("gpu", "Texture::ReadTrap");
@ -179,6 +191,9 @@ namespace skyline::gpu {
if (!lock)
return false;
if (texture->cycle)
return false;
texture->SynchronizeGuest(false, true); // We can skip trapping since the caller will do it
return true;
}, [weakThis] {
@ -200,6 +215,10 @@ namespace skyline::gpu {
std::unique_lock lock{*texture, std::try_to_lock};
if (!lock)
return false;
if (texture->cycle)
return false;
texture->SynchronizeGuest(true, true); // We need to assume the texture is dirty since we don't know what the guest is writing
return true;
});