wine/wine-staging
0
0
Fork 0
mirror of https://gitlab.winehq.org/wine/wine-staging.git synced 2024-11-21 16:46:54 -08:00
Code Issues Packages Projects Releases Wiki Activity

eventfd_synchronization: Cherry-pick some fixes.

Browse Source
This commit is contained in:
Zebediah Figura 2019-10-02 20:37:48 -05:00
parent 6be963ebfd
commit cf04b8d6ac
5 changed files with 335 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
patches/eventfd_synchronization/0085-server-Only-signal-the-APC-fd-for-user-APCs.patch Normal file
View File

@ -0,0 +1,26 @@
From 9e4df70f7282b04849153b3fa2edf15dc24eaf4f Mon Sep 17 00:00:00 2001
From: Zebediah Figura <zfigura@codeweavers.com>
Date: Tue, 23 Jul 2019 18:39:06 -0500
Subject: [PATCH] server: Only signal the APC fd for user APCs.
Otherwise we might incorrectly return WAIT_IO_COMPLETION to the user when a system APC runs.
---
server/thread.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/thread.c b/server/thread.c
index fc751c2cb..2e77e5ff2 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -1057,7 +1057,7 @@ static int queue_apc( struct process *process, struct thread *thread, struct thr
{
wake_thread( thread );
- if (do_esync())
+ if (do_esync() && queue == &thread->user_apc)
esync_wake_fd( thread->esync_apc_fd );
}
--
2.23.0

43
patches/eventfd_synchronization/0086-ntdll-Check-the-APC-fd-first.patch Normal file
View File

@ -0,0 +1,43 @@
From 836f1b6b0560bd178efb8d52900b4b136f87ae30 Mon Sep 17 00:00:00 2001
From: Zebediah Figura <zfigura@codeweavers.com>
Date: Tue, 23 Jul 2019 17:22:20 -0500
Subject: [PATCH] ntdll: Check the APC fd first.
---
dlls/ntdll/esync.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/dlls/ntdll/esync.c b/dlls/ntdll/esync.c
index fc621ccfb..0adb4ad77 100644
--- a/dlls/ntdll/esync.c
+++ b/dlls/ntdll/esync.c
@@ -1046,6 +1046,14 @@ static NTSTATUS __esync_wait_objects( DWORD count, const HANDLE *handles,
ret = do_poll( fds, pollcount, timeout ? &end : NULL );
if (ret > 0)
{
+ /* We must check this first! The server may set an event that
+ * we're waiting on, but we need to return STATUS_USER_APC. */
+ if (alertable)
+ {
+ if (fds[pollcount - 1].revents & POLLIN)
+ goto userapc;
+ }
+
/* Find out which object triggered the wait. */
for (i = 0; i < count; i++)
{
@@ -1089,11 +1097,6 @@ static NTSTATUS __esync_wait_objects( DWORD count, const HANDLE *handles,
return count - 1;
}
}
- if (alertable)
- {
- if (fds[i++].revents & POLLIN)
- goto userapc;
- }
/* If we got here, someone else stole (or reset, etc.) whatever
* we were waiting for. So keep waiting. */
--
2.23.0

52
patches/eventfd_synchronization/0087-ntdll-esync-Lock-accessing-the-shm_addrs-array.patch Normal file
View File

@ -0,0 +1,52 @@
From c1804983dc8e9509c088c35914212cda1bd5a48a Mon Sep 17 00:00:00 2001
From: Zebediah Figura <zfigura@codeweavers.com>
Date: Wed, 7 Aug 2019 17:14:54 -0500
Subject: [PATCH] ntdll/esync: Lock accessing the shm_addrs array.
---
dlls/ntdll/esync.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/esync.c b/dlls/ntdll/esync.c
index 0adb4ad77..2f030c141 100644
--- a/dlls/ntdll/esync.c
+++ b/dlls/ntdll/esync.c
@@ -155,10 +155,22 @@ void esync_init(void)
shm_addrs_size = 128;
}
+static RTL_CRITICAL_SECTION shm_addrs_section;
+static RTL_CRITICAL_SECTION_DEBUG shm_addrs_debug =
+{
+ 0, 0, &shm_addrs_section,
+ { &shm_addrs_debug.ProcessLocksList, &shm_addrs_debug.ProcessLocksList },
+ 0, 0, { (DWORD_PTR)(__FILE__ ": shm_addrs_section") }
+};
+static RTL_CRITICAL_SECTION shm_addrs_section = { &shm_addrs_debug, -1, 0, 0, 0, 0 };
+
static void *get_shm( unsigned int idx )
{
int entry = (idx * 8) / pagesize;
int offset = (idx * 8) % pagesize;
+ void *ret;
+
+ RtlEnterCriticalSection(&shm_addrs_section);
if (entry >= shm_addrs_size)
{
@@ -180,7 +192,11 @@ static void *get_shm( unsigned int idx )
munmap( addr, pagesize ); /* someone beat us to it */
}
- return (void *)((unsigned long)shm_addrs[entry] + offset);
+ ret = (void *)((unsigned long)shm_addrs[entry] + offset);
+
+ RtlLeaveCriticalSection(&shm_addrs_section);
+
+ return ret;
}
/* We'd like lookup to be fast. To that end, we use a static list indexed by handle.
--
2.23.0

206
patches/eventfd_synchronization/0088-ntdll-Get-rid-of-the-per-event-spinlock-for-auto-res.patch Normal file
View File

@ -0,0 +1,206 @@
From c41dc5b8c422be3914cd31c239ec586a091b8a3b Mon Sep 17 00:00:00 2001
From: Zebediah Figura <zfigura@codeweavers.com>
Date: Mon, 10 Jun 2019 11:25:34 -0400
Subject: [PATCH] ntdll: Get rid of the per-event spinlock for auto-reset
events.
It's not necessary. Much like semaphores, the shm state is just a hint.
---
dlls/ntdll/esync.c | 74 +++++++++++++++++++++++++++++++++++-----------
server/esync.c | 32 +++++++++++++-------
2 files changed, 78 insertions(+), 28 deletions(-)
diff --git a/dlls/ntdll/esync.c b/dlls/ntdll/esync.c
index 2f030c141..87f303403 100644
--- a/dlls/ntdll/esync.c
+++ b/dlls/ntdll/esync.c
@@ -570,6 +570,14 @@ static inline void small_pause(void)
* problem at all.
*/
+/* Removing this spinlock is harder than it looks. esync_wait_objects() can
+ * deal with inconsistent state well enough, and a race between SetEvent() and
+ * ResetEvent() gives us license to yield either result as long as we act
+ * consistently, but that's not enough. Notably, esync_wait_objects() should
+ * probably act like a fence, so that the second half of esync_set_event() does
+ * not seep past a subsequent reset. That's one problem, but no guarantee there
+ * aren't others. */
+
NTSTATUS esync_set_event( HANDLE handle, LONG *prev )
{
static const uint64_t value = 1;
@@ -583,21 +591,36 @@ NTSTATUS esync_set_event( HANDLE handle, LONG *prev )
if ((ret = get_object( handle, &obj ))) return ret;
event = obj->shm;
- /* Acquire the spinlock. */
- while (interlocked_cmpxchg( &event->locked, 1, 0 ))
- small_pause();
+ if (obj->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Acquire the spinlock. */
+ while (interlocked_cmpxchg( &event->locked, 1, 0 ))
+ small_pause();
+ }
+
+ /* For manual-reset events, as long as we're in a lock, we can take the
+ * optimization of only calling write() if the event wasn't already
+ * signaled.
+ *
+ * For auto-reset events, esync_wait_objects() must grab the kernel object.
+ * Thus if we got into a race so that the shm state is signaled but the
+ * eventfd is unsignaled (i.e. reset shm, set shm, set fd, reset fd), we
+ * *must* signal the fd now, or any waiting threads will never wake up. */
/* Only bother signaling the fd if we weren't already signaled. */
- if (!(current = interlocked_xchg( &event->signaled, 1 )))
+ if (!(current = interlocked_xchg( &event->signaled, 1 )) || obj->type == ESYNC_AUTO_EVENT)
{
if (write( obj->fd, &value, sizeof(value) ) == -1)
- return FILE_GetNtStatus();
+ ERR("write: %s\n", strerror(errno));
}
if (prev) *prev = current;
- /* Release the spinlock. */
- event->locked = 0;
+ if (obj->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Release the spinlock. */
+ event->locked = 0;
+ }
return STATUS_SUCCESS;
}
@@ -615,21 +638,34 @@ NTSTATUS esync_reset_event( HANDLE handle, LONG *prev )
if ((ret = get_object( handle, &obj ))) return ret;
event = obj->shm;
- /* Acquire the spinlock. */
- while (interlocked_cmpxchg( &event->locked, 1, 0 ))
- small_pause();
+ if (obj->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Acquire the spinlock. */
+ while (interlocked_cmpxchg( &event->locked, 1, 0 ))
+ small_pause();
+ }
- /* Only bother signaling the fd if we weren't already signaled. */
- if ((current = interlocked_xchg( &event->signaled, 0 )))
+ /* For manual-reset events, as long as we're in a lock, we can take the
+ * optimization of only calling read() if the event was already signaled.
+ *
+ * For auto-reset events, we have no guarantee that the previous "signaled"
+ * state is actually correct. We need to leave both states unsignaled after
+ * leaving this function, so we always have to read(). */
+ if ((current = interlocked_xchg( &event->signaled, 0 )) || obj->type == ESYNC_AUTO_EVENT)
{
- /* we don't care about the return value */
- read( obj->fd, &value, sizeof(value) );
+ if (read( obj->fd, &value, sizeof(value) ) == -1 && errno != EWOULDBLOCK && errno != EAGAIN)
+ {
+ ERR("read: %s\n", strerror(errno));
+ }
}
if (prev) *prev = current;
- /* Release the spinlock. */
- event->locked = 0;
+ if (obj->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Release the spinlock. */
+ event->locked = 0;
+ }
return STATUS_SUCCESS;
}
@@ -844,8 +880,9 @@ static void update_grabbed_object( struct esync *obj )
else if (obj->type == ESYNC_AUTO_EVENT)
{
struct event *event = obj->shm;
- /* We don't have to worry about a race between this and read(), for
- * reasons described near esync_set_event(). */
+ /* We don't have to worry about a race between this and read(), since
+ * this is just a hint, and the real state is in the kernel object.
+ * This might already be 0, but that's okay! */
event->signaled = 0;
}
}
@@ -1094,6 +1131,7 @@ static NTSTATUS __esync_wait_objects( DWORD count, const HANDLE *handles,
}
else
{
+ /* FIXME: Could we check the poll or shm state first? Should we? */
if ((size = read( fds[i].fd, &value, sizeof(value) )) == sizeof(value))
{
/* We found our object. */
diff --git a/server/esync.c b/server/esync.c
index 4521993d4..84d0951cb 100644
--- a/server/esync.c
+++ b/server/esync.c
@@ -396,9 +396,12 @@ void esync_set_event( struct esync *esync )
if (debug_level)
fprintf( stderr, "esync_set_event() fd=%d\n", esync->fd );
- /* Acquire the spinlock. */
- while (interlocked_cmpxchg( &event->locked, 1, 0 ))
- small_pause();
+ if (esync->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Acquire the spinlock. */
+ while (interlocked_cmpxchg( &event->locked, 1, 0 ))
+ small_pause();
+ }
if (!interlocked_xchg( &event->signaled, 1 ))
{
@@ -406,8 +409,11 @@ void esync_set_event( struct esync *esync )
perror( "esync: write" );
}
- /* Release the spinlock. */
- event->locked = 0;
+ if (esync->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Release the spinlock. */
+ event->locked = 0;
+ }
}
void esync_reset_event( struct esync *esync )
@@ -421,9 +427,12 @@ void esync_reset_event( struct esync *esync )
if (debug_level)
fprintf( stderr, "esync_reset_event() fd=%d\n", esync->fd );
- /* Acquire the spinlock. */
- while (interlocked_cmpxchg( &event->locked, 1, 0 ))
- small_pause();
+ if (esync->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Acquire the spinlock. */
+ while (interlocked_cmpxchg( &event->locked, 1, 0 ))
+ small_pause();
+ }
/* Only bother signaling the fd if we weren't already signaled. */
if (interlocked_xchg( &event->signaled, 0 ))
@@ -432,8 +441,11 @@ void esync_reset_event( struct esync *esync )
read( esync->fd, &value, sizeof(value) );
}
- /* Release the spinlock. */
- event->locked = 0;
+ if (esync->type == ESYNC_MANUAL_EVENT)
+ {
+ /* Release the spinlock. */
+ event->locked = 0;
+ }
}
DECL_HANDLER(create_esync)
--
2.23.0

8
patches/patchinstall.sh
View File

@ -3824,6 +3824,10 @@ if test "$enable_eventfd_synchronization" -eq 1; then
patch_apply eventfd_synchronization/0082-ntdll-server-Check-the-value-of-WINEESYNC-instead-of.patch
patch_apply eventfd_synchronization/0083-esync-Update-README.patch
patch_apply eventfd_synchronization/0084-server-Use-default_fd_get_esync_fd-for-directory-cha.patch
patch_apply eventfd_synchronization/0085-server-Only-signal-the-APC-fd-for-user-APCs.patch
patch_apply eventfd_synchronization/0086-ntdll-Check-the-APC-fd-first.patch
patch_apply eventfd_synchronization/0087-ntdll-esync-Lock-accessing-the-shm_addrs-array.patch
patch_apply eventfd_synchronization/0088-ntdll-Get-rid-of-the-per-event-spinlock-for-auto-res.patch
(
printf '%s\n' '+ { "Zebediah Figura", "configure: Check for sys/eventfd.h, ppoll(), and shm_open().", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "server: Create server objects for eventfd-based synchronization objects.", 1 },';
@ -3909,6 +3913,10 @@ if test "$enable_eventfd_synchronization" -eq 1; then
printf '%s\n' '+ { "Zebediah Figura", "ntdll, server: Check the value of WINEESYNC instead of just the presence.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "esync: Update README.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "server: Create esync file descriptors for true file objects and use them for directory change notifications.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "server: Only signal the APC fd for user APCs.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "ntdll: Check the APC fd first.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "ntdll/esync: Lock accessing the shm_addrs array.", 1 },';
printf '%s\n' '+ { "Zebediah Figura", "ntdll: Get rid of the per-event spinlock for auto-reset events.", 1 },';
) >> "$patchlist"
fi