-
Notifications
You must be signed in to change notification settings - Fork 186
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
RFC: Never touch wineserver or the Steam client (do not merge, revision planned) #76
base: master
Are you sure you want to change the base?
Conversation
I wonder whether this would be better handled via a default blacklist option (when not explicitly set in a config file, and also update the example config to include them with an explanation), rather than hardcoding it, so that the behaviour can be disabled. I can't think of any reasonable use case for entering game mode when wineserver is running, but it's possible that some users may want it when they have Steam running (the built-in functionality doesn't really make sense, but they may wish to do something with a custom script). |
Does the default blacklist handle partial paths correctly? |
Look here: gamemode/daemon/daemon_config.c Line 360 in 0d179e5
It does a simple test to check for the occurrence of a match in the exe path. But for correctly handling Wine and Steam processes, we need to match some of the entries with the end of the string, especially EDIT: Updated to correct line number. |
But I'm open to rebase and improve the blacklist handler instead... |
My other pull request provides the infrastructure to do selective filtering of wine processes. That could act as a base for this idea. |
Yes, I think that'd be best. |
In spe of revisiting this: What's your suggestion on blacklisting wine processes by default? With my other PR we'd be able to use blacklists like "windows/system32", "windows/syswow64", and "/wineserver". It could be done as an example only (and many people would miss this optimization), or as a default blacklist hard-coded into the daemon binary unless overwritten by a blacklist config (which violates the principle of least surprise). Because of this, the best option would probably be an on/off switch like "protonsupport=yes/no" or "winesupport=yes/no", defaulting to yes. And that adds the relevant blacklist entries in. |
I like the idea of a winesupport option, which adds the blacklisting on top of any blacklist specified by the user. Doesn't handle the Steam case, but I think if users want to preload it into the Steam client rather than per-game, we could just document that they might want to manually blacklist it themselves depending on whether or not they actually want game mode to be active while Steam is running or just when a game is. |
So why not put this a level further and also add Since we are both in the same position now how to handle this, I would rebase this work onto my wine support branch and push it here once the other branch is merged. |
@aejsmith So I'm back with a fresh set of ideas. My currently idea (and I started working on an implementation) is to use so-called "filter sets". A filter set is a section like Here's an example of my idea: ; Defines default blacklist sets for easy blacklisting of common software
; which should not be managed by GameMode. The [set:default] section is evaluated
; if your custom gamemode.ini does not configure "filter-sets". Filter sets can
; be nested by repeating "filter-sets" within the section.
; Each set section evaluated doesn't overwrite a blacklist but adds the
; entries to the blacklist. Whitelists can also be used.
[set:default]
filter-sets = launchers wine
; Whitelist sets can also be used. If you define a whitelist everything else
; is blacklisted. This section is not used by default. This example would
; whitelist any game in your default Steam library.
[set:steamlib]
whitelist = /steam/SteamApps/common/
; Common shell wrappers that would mix up the desired results, we do
; not want these to leak scheduler settings into subprocesses.
[set:shellwrappers]
blacklist =
/bin/ionice
/bin/schedtool
/bin/env
/bin/python
; Game launchers should not be managed by GameMode even if you started
; the complete launcher under GameMode. Expectations are rather that
; each game would run under GameMode individually then. This blacklist
; set enables this expected behavior. GameMode can detect if the launcher
; leaks unwanted scheduler settings into its subprocesses and would
; complain in the log about this.
[set:launchers]
filter-sets = steam
[set:steam]
filter-sets = shellwrappers ; used by SteamPlay (Proton launcher)
blacklist =
/steamerrorreporter
/steamwebhelper
/ubuntu12_32/steam
; Wine starts several subprocesses and manages these by itself.
; We should not interfere with that as it may result in priority
; inversion between the game and the Windows API. GameMode is still
; able to correctly evaluate the exe paths of programs running under
; Wine and ignores the Wine loaders until the .exe file is loaded.
[set:wine]
blacklist =
/bin/ntlm_auth
/windows/system32/
/windows/syswow64/
/wineserver |
Sounds like a good plan. Keeps things configurable without hardcoding a bunch of process names/paths, and makes it easier to enable/disable behaviour for Steam etc. |
This looks quite stale. Is anyone planning to follow-up? |
Yes, I will follow-up but my whole development env is currently not usable. This is fixed in a few weeks. |
b05d29b
to
a7c98bb
Compare
a7c98bb
to
eff04f5
Compare
eff04f5
to
6b688be
Compare
Hey, it looks like the PR needs a new rebase. |
I'll look into it, give me a few days... |
Just a friendly ping in case you forgot about it :) |
6b688be
to
1b79ab3
Compare
Thanks... Yeah, I actually forgot about it. ;-) |
I just tested your rebased PR, is this the correct behaviour ? I see all the wine processes with the same priority. |
This is expected as designed at this point. The PR is mainly about detecting the executable running in the wine container, so you could actually blacklist based on the exe name. It also ignores wineserver via hardcoded exception because wineserver should not be touched: It is sensible to priority changes and may bring it's own handler for adjusting its main thread (staging patches needed). Fiddling with wineserver can result in priority inversions. So if you didn't expect all the internal wine processes being changed to another priority, you may want to blacklist |
Probably because you used a shell wrapper to start wine and applied |
I used Blacklisting worked. EDIT: replacing wine64 by wine worked as well. Could there be something gamemode do when using wine64 instead of wine? |
There's an improved black/whitelist extension planned which also supports sets which can be easily distributed with gamemode. But I'm currently lacking a lot of time. |
Nice and ok, this is understandable. |
The basic idea is outlined here: #76 (comment) |
1b79ab3
to
4d99e09
Compare
4d99e09
to
3aba2f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase to 1.6
Actually, it wasn't planned to only use this code by the wine detection, there's more code in my queue that uses this. This reverts commit 42a803c.
This enables us to reuse the open function for different purposes. Signed-off-by: Kai Krakow <[email protected]>
With the arrival of SteamPlay Proton, we probably don't want to touch wineserver at all. With staging patches, wineserver has it's own means of gaining realtime priority and handling spawned processes. Optimally, gamemode support would be integrated right into Proton itself by changing the process spawning functions of wine when launching the actual game exe. I'm currently working on such an idea. This could be extended to ignore other well-known binaries which should never be handled by gamemode (i.e. shells of scripted game launchers). Signed-off-by: Kai Krakow <[email protected]>
If you want to run your whole Steam client under gamemode to not adjust each game individually, we certainly should leave the Steam client processes alone as we don't want them to take resources away from the game. Otherwise, Steam client processes may run with `SCHED_ISO`. Closes: FeralInteractive#67 Signed-off-by: Kai Krakow <[email protected]>
Let's document the changes to automatically handle Steam, SteamPlay, and Wine. Also, give an example how to run the Steam client or Wine games in GameMode. Signed-off-by: Kai Krakow <[email protected]>
3aba2f2
to
fcdae41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep subprojects
accidentally removed in revert commit
With the arrival of SteamPlay Proton, we probably don't want to touch
wineserver at all. With staging patches, wineserver has it's own means
of gaining realtime priority and handling spawned processes.
Optimally, gamemode support would be integrated right into Proton itself
by changing the process spawning functions of wine when launching the
actual game exe. I'm currently working on such an idea.
This could be extended to ignore other well-known binaries which should
never be handled by gamemode (i.e. shells of scripted game launchers).
The following commits add support for properly handling handling Wine
games and the whole Steam client, including examples of how to do it.
Signed-off-by: Kai Krakow [email protected]