sys_game_watchdog minor fixups #12827
Conversation
elad335
force-pushed
the
watchdog
branch
2 times, most recently
from
5147c91
to
a6eb829
Compare
elad335
force-pushed
the
watchdog
branch
2 times, most recently
from
d05bd2d
to
3b0637a
Compare
rpcs3/Emu/Cell/lv2/sys_game.cpp
Outdated
| { | ||
| if (Emu.IsStopped() || get_timestamp() - watchdog_last_clear > timeout * 1000000) | ||
| if (Emu.IsPaused()) |
There was a problem hiding this comment.
Why can't we just use get_system_time() - Emu.GetPauseTime() to offset the paused time?
There was a problem hiding this comment.
I rather the time be observed by the thread itself, GetPauseTime has a theoretical thread safety issue because Emu.Pause() does not wait for all threads to actually pause.
| auto watchdog = [=]() | ||
| { | ||
| while (!watchdog_stopped) | ||
| constexpr u64 sleep_time = 50'000; |
There was a problem hiding this comment.
I wonder how this is decided.
In the log, I saw the watchdog was cleared each frame (sys_game_watchdog_clear() was called per 16.x ms).
There was a problem hiding this comment.
But this API has 1s resolution anyway, why it matters?
elad335
force-pushed
the
watchdog
branch
2 times, most recently
from
77ca8d1
to
b848c11
Compare
| } | ||
| }; | ||
|
|
||
| error_code _sys_game_watchdog_start(u32 timeout) |
There was a problem hiding this comment.
Could you help me confirm if the param 1 is indeed u32?
I was unable to check as in Ghidra it was shown as undefined, and the psdevwiki just said it's int time(r).
There was a problem hiding this comment.
Would you mind sending me the disassembly of sys_game syscalls so that I might be able to implement them more accurately in the future? Thank you.
There was a problem hiding this comment.
IDA file. lv2_446_+_bss_data.zip
elad335
force-pushed
the
watchdog
branch
2 times, most recently
from
f49dc89
to
6d48e15
Compare
rpcs3/Emu/Cell/lv2/sys_game.cpp
Outdated
| { | ||
| bool needs_restart = false; | ||
| bool active = false; | ||
| char pad[sizeof(u64) - sizeof(bool) * 2]{}; |
There was a problem hiding this comment.
Where's this (char pad[]) used?
There was a problem hiding this comment.
It's so atomic compare&swap operations have defined result because they may fail with implicit padding bytes in some cases.
|
I'll test it with the actual game later. |
rpcs3/Emu/Cell/lv2/sys_game.cpp
Outdated
| { | ||
| if (Emu.IsStopped() || get_timestamp() - watchdog_last_clear > timeout * 1000000) | ||
| if (Emu.IsPaused()) |
There was a problem hiding this comment.
The watchdog wasn't triggered when the game status was "frozen" because Emu.IsPaused() includes not only paused, but also frozen, ready, and starting. Shouldn't the watchdog attempt to restart the game when the game is no longer responding?
The watchdog worked well when the game intentionally stopped calling sys_game_watchdog_clear() (but the game wasn't actually frozen).
However, the watchdog wasn't triggered when the game was frozen unexpectedly (F {PPU[0x1000000] Thread (main_thread) [0x005130c8]} VM: Access violation reading location 0x0 (unmapped memory))
There was a problem hiding this comment.
Hmm, I need to think what to do.
There was a problem hiding this comment.
Great. It was triggered in both cases.
Thank you for your hard work!
|
@elad335 By the way, is it safe to remove the implemented sys_game syscalls from /Emu/Cell/Modules/sys_game_.cpp? |
|
No because they are not syscalls. |
Requesting @brian218 to review.