Skip to content

Instantly share code, notes, and snippets.

@ivyl
Created November 1, 2021 13:11
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ivyl/93107ffef32b2e8816d1176286afdd12 to your computer and use it in GitHub Desktop.
Save ivyl/93107ffef32b2e8816d1176286afdd12 to your computer and use it in GitHub Desktop.
From 0aab2bbbe2f0b31ff725739621fbe0f43803ead2 Mon Sep 17 00:00:00 2001
From: Arkadiusz Hiler <ahiler@codeweavers.com>
Date: Mon, 1 Nov 2021 14:25:42 +0200
Subject: [PATCH] HAX: Disable fsync if we detect the old futex2 patches.
futex_waitv() has been accepted into locking/core branch of kernel.org's
tip repo and has a fixed syscall number.
Even though the syscall number matches (449) there were changes that
result in 100% CPU utilization at all times when running with the old,
downstream version of the futex2 patches.
The new patches do not come with the sysfs entries so we can use that
for the detection.
Fixes: https://github.com/ValveSoftware/wine/pull/128
---
dlls/ntdll/unix/fsync.c | 8 ++++++++
server/fsync.c | 9 +++++++++
2 files changed, 17 insertions(+)
diff --git a/dlls/ntdll/unix/fsync.c b/dlls/ntdll/unix/fsync.c
index 1291f3171a1..8f07ed59ae0 100644
--- a/dlls/ntdll/unix/fsync.c
+++ b/dlls/ntdll/unix/fsync.c
@@ -164,6 +164,14 @@ int do_fsync(void)
if (do_fsync_cached == -1)
{
+ FILE *f;
+ if ((f = fopen( "/sys/kernel/futex2/wait", "r" )))
+ {
+ fclose(f);
+ do_fsync_cached = 0;
+ return do_fsync_cached;
+ }
+
syscall( __NR_futex_waitv, NULL, 0, 0, NULL, 0 );
do_fsync_cached = getenv("WINEFSYNC") && atoi(getenv("WINEFSYNC")) && errno != ENOSYS;
if (getenv("WINEFSYNC_SPINCOUNT"))
diff --git a/server/fsync.c b/server/fsync.c
index 237e8f2b4a4..45760bc0320 100644
--- a/server/fsync.c
+++ b/server/fsync.c
@@ -60,6 +60,15 @@ int do_fsync(void)
if (do_fsync_cached == -1)
{
+ FILE *f;
+ if ((f = fopen( "/sys/kernel/futex2/wait", "r" )))
+ {
+ fclose(f);
+ do_fsync_cached = 0;
+ fprintf( stderr, "fsync: old futex2 patches detected, disabling.\n" );
+ return do_fsync_cached;
+ }
+
syscall( __NR_futex_waitv, 0, 0, 0, 0, 0);
do_fsync_cached = getenv("WINEFSYNC") && atoi(getenv("WINEFSYNC")) && errno != ENOSYS;
}
--
2.33.1
@kakra
Copy link

kakra commented Nov 1, 2021

Do futex2 and the merged futex use the same syscall nr?

@ivyl
Copy link
Author

ivyl commented Nov 1, 2021

Even though the syscall number matches (449) there were changes that
result in 100% CPU utilization at all times when running with the old,
downstream version of the futex2 patches.

@kakra that's kinda mentioned in the commit message, but let me elaborate.

The old futex2 patches came with sysfs entries to get the syscall numbers for the new futex calls (waitv, wait and wake). On all the downstream kernels I've tested waitv used 449, but that's not guaranteed. The version that landed in tip's locking/core has only waitv that also uses 449. The two other functions were dropped. The format of the structure also has changed, e.g. val switched places with uaddr.

@kakra
Copy link

kakra commented Nov 1, 2021

So the sycallnr 449 is just duplicate by coincidence... And we could continue to support both patchsets in Proton if we moved futex2 to a higher syscallnr base? I mean, by explicitly avoiding the conflict...

@ivyl
Copy link
Author

ivyl commented Nov 1, 2021

If you are willing to recompile the kernel to change the syscall number you can as well recompile it with the new version of the patch that the developer has provided in the linked PR.

I know we could theoretically use sysfs to make the distinction, just like this patch does. Handling the struct layout changes would make the code quite ugly though. All that to support something that was never upstream and is not significantly better than esync... I don't think it's worth it.

@kakra
Copy link

kakra commented Nov 1, 2021

Ah ok I think I understand better where the problems are now: So in-kernel, both patchsets are more or less mutually exclusive (except you want to jump hoops and loops to handle the struct layouts - which we don't want to do).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment