Fix VRAM leak when monitors are off#3910
Conversation
c4ea5b0 to
67a53d8
Compare
|
well this clearly looks kinda cursed with that check and pub(crate) |
67a53d8 to
2678cc1
Compare
|
Refactored: both functions moved to Heads up: with the throttle now backend-agnostic, |
|
I'm gonna have to take a good look at this and at surrounding code to verify that it's right. Sometime in the future |
|
Thank you. |
2678cc1 to
3cfd6b2
Compare
|
Tested on lenovo t490/i915 + arch + niri-git + 3910. Still observing the leak leading to eventual overnight OOMs. First time observed on Dec 12 (though it might be that I wasn't updating the system for up to 2 weeks at the time, take it as an approx date). Powering off monitors frequently (Mod+Shift+P { power-off-monitors; }). Tried to downgrade to the oldest available locally cached version (25.08-2), did not helped, |
|
What helped: |
3cfd6b2 to
9b10d10
Compare
Stop driving the redraw loop while monitors are off: schedule the estimated-vblank throttle, skip send_frame_callbacks and the fallback timer, and clear unfinished_animations_remain. Refs niri-wm#3295.
9b10d10 to
164c957
Compare
|
@sys-rq Thanks for the testing and the diagnosis - all three paths you identified check out against the code, and they explain why my original patch didn't close the leak (it only stopped the per-commit busy-loop, not the timer-driven re-queuing). Folded all three into this PR (force-pushed, see updated description):
If you have time, a re-test on the new revision would be much appreciated. |
|
Per my own tests with AMD iGPU, commit 3cfd6b2 fixed the VRAM leak related to monitor powered off. In the next hours/days I'll test the latest commit |
|
looks promising, tested for 1h with monitor off and, just in case, locked but turned on, |
|
Thanks for re-testing - good to have confirmation on the hardware where the leak was reproducible. Appreciate the legwork on the original diagnosis too. |
Per discussion in #3295: with monitors off, several paths kept driving the redraw loop and leaking memory.
This patch breaks the loop by:
Tty::renderschedules onNoDamage, so commit-drivenqueue_redrawdoesn't busy-loop.send_frame_callbackswhilemonitors_active == false- we never present, so inviting clients to commit just queues buffers that go nowhere.unfinished_animations_remainin the inactive-monitors branch so the estimated-vblank timer doesn't keep re-queuing redraws at refresh-rate cadence.The throttle helpers moved from
Ttytoimpl Niriso the inactive-monitors call site doesn't need a backend-type check.Credit to @sys-rq for empirical testing on i915 + Lenovo T490 that surfaced the three additional drivers; the original patch only addressed the per-commit busy-loop.
Testing
cargo check/cargo clippy --all --all-targets/cargo +nightly fmt --all -- --checkclean.cargo test --all --exclude niri-visual-tests- 217 tests pass.Refs #3295.