gamemode icon indicating copy to clipboard operation
gamemode copied to clipboard

RFC: Never touch wineserver or the Steam client (do not merge, revision planned)

Open kakra opened this issue 7 years ago • 26 comments

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]

kakra avatar Sep 24 '18 18:09 kakra

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).

aejsmith avatar Sep 29 '18 08:09 aejsmith

Does the default blacklist handle partial paths correctly?

kakra avatar Sep 29 '18 08:09 kakra

Look here: https://github.com/FeralInteractive/gamemode/blob/0d179e520a9bdd6a84fc160d1d87c530c76b99f1/daemon/daemon_config.c#L360

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 steam is not very unique. I don't want to match processes having "steam" in them because game exes may use that (e.g. some games ship exe names with "steam" in them).

EDIT: Updated to correct line number.

kakra avatar Sep 29 '18 08:09 kakra

But I'm open to rebase and improve the blacklist handler instead...

kakra avatar Sep 29 '18 08:09 kakra

My other pull request provides the infrastructure to do selective filtering of wine processes. That could act as a base for this idea.

kakra avatar Sep 29 '18 16:09 kakra

But I'm open to rebase and improve the blacklist handler instead...

Yes, I think that'd be best.

aejsmith avatar Oct 02 '18 13:10 aejsmith

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.

kakra avatar Oct 03 '18 02:10 kakra

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.

aejsmith avatar Oct 03 '18 08:10 aejsmith

So why not put this a level further and also add steamsupport=yes which does the same for steam? And then maybe protonsupport=yes/steamplaysupport=yes which combines both options.

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.

kakra avatar Oct 03 '18 11:10 kakra

@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 [filter] but stand-alone. The filter section would then allow to include filter sets (and that works recursively, as sets could also include other sets). So resolve the recursion, a two-pass resolver is used so we don't get into fancy endless recursion scenarios.

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

kakra avatar Oct 12 '18 19:10 kakra

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.

aejsmith avatar Oct 17 '18 08:10 aejsmith

This looks quite stale. Is anyone planning to follow-up?

nanonyme avatar Aug 01 '19 06:08 nanonyme

Yes, I will follow-up but my whole development env is currently not usable. This is fixed in a few weeks.

kakra avatar Aug 01 '19 08:08 kakra

Just a resync of my branch with current master because so much has changed... None of the commits changed functionality over my old version. This is purely for people using this as a patch: They now can for a current version of GameMode. :-)

@aejsmith However, you may want to cherry-pick a5390d81b0aaea2a85e1996d3c9542b6847eaaf3.

kakra avatar Oct 14 '19 21:10 kakra

Hey, it looks like the PR needs a new rebase.

terencode avatar Jan 29 '20 21:01 terencode

I'll look into it, give me a few days...

kakra avatar Jan 30 '20 17:01 kakra

Just a friendly ping in case you forgot about it :)

terencode avatar Apr 14 '20 00:04 terencode

Just a friendly ping in case you forgot about it :)

Thanks... Yeah, I actually forgot about it. ;-)

kakra avatar Apr 14 '20 08:04 kakra

I just tested your rebased PR, is this the correct behaviour ? log.txt

I see all the wine processes with the same priority.

terencode avatar Apr 17 '20 14:04 terencode

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 windows/system32 (which you can do now thanks to this PR).

kakra avatar Apr 17 '20 15:04 kakra

Ah ok thanks. So if wineserver isn't touched why does it have the renice value I set? image

terencode avatar Apr 17 '20 15:04 terencode

So if wineserver isn't touched why does it have the renice value I set?

Probably because you used a shell wrapper to start wine and applied LD_PRELOAD to it. The result is: The wrapper becomes reniced and its children will inherit it. You should follow advice of the log and blacklist /usr/bin/wine64.

kakra avatar Apr 17 '20 15:04 kakra

I used gamemoderun wine64 program is that the same?

Blacklisting worked. Do you plan to include more things into this PR with the "draft" mark?

EDIT: replacing wine64 by wine worked as well. Could there be something gamemode do when using wine64 instead of wine?

terencode avatar Apr 17 '20 16:04 terencode

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.

kakra avatar Apr 17 '20 17:04 kakra

Nice and ok, this is understandable.

terencode avatar Apr 17 '20 18:04 terencode

Nice and ok, this is understandable.

The basic idea is outlined here: https://github.com/FeralInteractive/gamemode/pull/76#issuecomment-429436159

kakra avatar Apr 17 '20 18:04 kakra