[PATCH 0/6] MR5940: server, win32u: Use the message queue shared memory to read the queue status.
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/protocol.def | 2 ++ server/queue.c | 60 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index 33fd7a96e03..6b0ab8ae466 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -916,6 +916,8 @@ typedef volatile struct typedef volatile struct { int hooks_count[WH_MAX - WH_MIN + 2]; /* active hooks count */ + unsigned int wake_mask; /* wakeup mask */ + unsigned int changed_mask; /* changed wakeup mask */ } queue_shm_t; typedef volatile union diff --git a/server/queue.c b/server/queue.c index 53306f46793..d79b4839733 100644 --- a/server/queue.c +++ b/server/queue.c @@ -126,9 +126,7 @@ struct msg_queue struct object obj; /* object header */ struct fd *fd; /* optional file descriptor to poll */ unsigned int wake_bits; /* wakeup bits */ - unsigned int wake_mask; /* wakeup mask */ unsigned int changed_bits; /* changed wakeup bits */ - unsigned int changed_mask; /* changed wakeup mask */ int paint_count; /* pending paint messages count */ int hotkey_count; /* pending hotkey messages count */ int quit_message; /* is there a pending quit message? */ @@ -303,9 +301,7 @@ static struct msg_queue *create_msg_queue( struct thread *thread, struct thread_ { queue->fd = NULL; queue->wake_bits = 0; - queue->wake_mask = 0; queue->changed_bits = 0; - queue->changed_mask = 0; queue->paint_count = 0; queue->hotkey_count = 0; queue->quit_message = 0; @@ -332,6 +328,8 @@ static struct msg_queue *create_msg_queue( struct thread *thread, struct thread_ SHARED_WRITE_BEGIN( queue->shared, queue_shm_t ) { memset( (void *)shared->hooks_count, 0, sizeof(shared->hooks_count) ); + shared->wake_mask = 0; + shared->changed_mask = 0; } SHARED_WRITE_END; @@ -642,7 +640,9 @@ void add_queue_hook_count( struct thread *thread, unsigned int index, int count /* check the queue status */ static inline int is_signaled( struct msg_queue *queue ) { - return ((queue->wake_bits & queue->wake_mask) || (queue->changed_bits & queue->changed_mask)); + const queue_shm_t *queue_shm = queue->shared; + return (queue->wake_bits & queue_shm->wake_mask) || + (queue->changed_bits & queue_shm->changed_mask); } /* set some queue bits */ @@ -1179,8 +1179,9 @@ static void msg_queue_remove_queue(struct object *obj, struct wait_queue_entry * static void msg_queue_dump( struct object *obj, int verbose ) { struct msg_queue *queue = (struct msg_queue *)obj; + const queue_shm_t *queue_shm = queue->shared; fprintf( stderr, "Msg queue bits=%x mask=%x\n", - queue->wake_bits, queue->wake_mask ); + queue->wake_bits, queue_shm->wake_mask ); } static int msg_queue_signaled( struct object *obj, struct wait_queue_entry *entry ) @@ -1203,8 +1204,14 @@ static int msg_queue_signaled( struct object *obj, struct wait_queue_entry *entr static void msg_queue_satisfied( struct object *obj, struct wait_queue_entry *entry ) { struct msg_queue *queue = (struct msg_queue *)obj; - queue->wake_mask = 0; - queue->changed_mask = 0; + const queue_shm_t *queue_shm = queue->shared; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_mask = 0; + shared->changed_mask = 0; + } + SHARED_WRITE_END; } static void msg_queue_destroy( struct object *obj ) @@ -1313,7 +1320,9 @@ static int check_queue_input_window( struct msg_queue *queue, user_handle_t wind void check_thread_queue_idle( struct thread *thread ) { struct msg_queue *queue = thread->queue; - if ((queue->wake_mask & QS_SMRESULT)) return; + const queue_shm_t *queue_shm = queue->shared; + + if ((queue_shm->wake_mask & QS_SMRESULT)) return; if (thread->process->idle_event) set_event( thread->process->idle_event ); } @@ -2937,14 +2946,29 @@ DECL_HANDLER(set_queue_mask) if (queue) { - queue->wake_mask = req->wake_mask; - queue->changed_mask = req->changed_mask; + const queue_shm_t *queue_shm = queue->shared; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_mask = req->wake_mask; + shared->changed_mask = req->changed_mask; + } + SHARED_WRITE_END; + reply->wake_bits = queue->wake_bits; reply->changed_bits = queue->changed_bits; if (is_signaled( queue )) { /* if skip wait is set, do what would have been done in the subsequent wait */ - if (req->skip_wait) queue->wake_mask = queue->changed_mask = 0; + if (req->skip_wait) + { + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_mask = 0; + shared->changed_mask = 0; + } + SHARED_WRITE_END; + } else wake_up( &queue->obj, 0 ); } } @@ -3108,6 +3132,7 @@ DECL_HANDLER(get_message) struct list *ptr; struct msg_queue *queue = get_current_queue(); user_handle_t get_win = get_user_full_handle( req->get_win ); + const queue_shm_t *queue_shm; unsigned int filter = req->flags >> 16; if (get_win && get_win != 1 && get_win != -1 && !get_user_object( get_win, USER_WINDOW )) @@ -3117,6 +3142,7 @@ DECL_HANDLER(get_message) } if (!queue) return; + queue_shm = queue->shared; /* check for any hardware internal message */ if (get_hardware_message( current, req->hw_id, get_win, WM_WINE_FIRST_DRIVER_MSG, @@ -3200,8 +3226,14 @@ DECL_HANDLER(get_message) } if (get_win == -1 && current->process->idle_event) set_event( current->process->idle_event ); - queue->wake_mask = req->wake_mask; - queue->changed_mask = req->changed_mask; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_mask = req->wake_mask; + shared->changed_mask = req->changed_mask; + } + SHARED_WRITE_END; + set_error( STATUS_PENDING ); /* FIXME */ } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 34 +++++++++++++++++++--------------- dlls/win32u/ntuser_private.h | 2 -- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 3099f5a6660..d8cb6f395b2 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2708,8 +2708,6 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) free( buffer ); if (res == STATUS_PENDING) { - thread_info->wake_mask = filter->mask & (QS_SENDMESSAGE | QS_SMRESULT); - thread_info->changed_mask = filter->mask; return 0; } if (res != STATUS_BUFFER_OVERFLOW) @@ -3017,6 +3015,23 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW return ret; } +/*********************************************************************** + * check_queue_masks + */ +static BOOL check_queue_masks( UINT wake_mask, UINT changed_mask ) +{ + struct object_lock lock = OBJECT_LOCK_INIT; + const queue_shm_t *queue_shm; + BOOL skip = FALSE; + UINT status; + + while ((status = get_shared_queue( &lock, &queue_shm )) == STATUS_PENDING) + skip = queue_shm->wake_mask == wake_mask && queue_shm->changed_mask == changed_mask; + + if (status) return FALSE; + return skip; +} + /*********************************************************************** * wait_objects * @@ -3025,14 +3040,11 @@ static DWORD wait_message( DWORD count, const HANDLE *handles, DWORD timeout, DW static DWORD wait_objects( DWORD count, const HANDLE *handles, DWORD timeout, DWORD wake_mask, DWORD changed_mask, DWORD flags ) { - struct user_thread_info *thread_info = get_user_thread_info(); - DWORD ret; - assert( count ); /* we must have at least the server queue */ flush_window_surfaces( TRUE ); - if (thread_info->wake_mask != wake_mask || thread_info->changed_mask != changed_mask) + if (!check_queue_masks( wake_mask, changed_mask )) { SERVER_START_REQ( set_queue_mask ) { @@ -3042,14 +3054,9 @@ static DWORD wait_objects( DWORD count, const HANDLE *handles, DWORD timeout, wine_server_call( req ); } SERVER_END_REQ; - thread_info->wake_mask = wake_mask; - thread_info->changed_mask = changed_mask; } - ret = wait_message( count, handles, timeout, changed_mask, flags ); - - if (ret != WAIT_TIMEOUT) thread_info->wake_mask = thread_info->changed_mask = 0; - return ret; + return wait_message( count, handles, timeout, changed_mask, flags ); } static HANDLE normalize_std_handle( HANDLE handle ) @@ -3314,7 +3321,6 @@ done: */ static void wait_message_reply( UINT flags ) { - struct user_thread_info *thread_info = get_user_thread_info(); HANDLE server_queue = get_server_queue_handle(); unsigned int wake_mask = QS_SMRESULT | ((flags & SMTO_BLOCK) ? 0 : QS_SENDMESSAGE); @@ -3331,8 +3337,6 @@ static void wait_message_reply( UINT flags ) } SERVER_END_REQ; - thread_info->wake_mask = thread_info->changed_mask = 0; - if (wake_bits & QS_SMRESULT) return; /* got a result */ if (wake_bits & QS_SENDMESSAGE) { diff --git a/dlls/win32u/ntuser_private.h b/dlls/win32u/ntuser_private.h index 44edadad8f8..e9d407efb14 100644 --- a/dlls/win32u/ntuser_private.h +++ b/dlls/win32u/ntuser_private.h @@ -108,8 +108,6 @@ struct user_thread_info { struct ntuser_thread_info client_info; /* Data shared with client */ HANDLE server_queue; /* Handle to server-side queue */ - DWORD wake_mask; /* Current queue wake mask */ - DWORD changed_mask; /* Current queue changed mask */ WORD message_count; /* Get/PeekMessage loop counter */ WORD hook_call_depth; /* Number of recursively called hook procs */ WORD hook_unicode; /* Is current hook unicode? */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/protocol.def | 2 ++ server/queue.c | 69 ++++++++++++++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/server/protocol.def b/server/protocol.def index 6b0ab8ae466..21e35204ed0 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -917,7 +917,9 @@ typedef volatile struct { int hooks_count[WH_MAX - WH_MIN + 2]; /* active hooks count */ unsigned int wake_mask; /* wakeup mask */ + unsigned int wake_bits; /* wakeup bits */ unsigned int changed_mask; /* changed wakeup mask */ + unsigned int changed_bits; /* changed wakeup bits */ } queue_shm_t; typedef volatile union diff --git a/server/queue.c b/server/queue.c index d79b4839733..4f58b795b7e 100644 --- a/server/queue.c +++ b/server/queue.c @@ -125,8 +125,6 @@ struct msg_queue { struct object obj; /* object header */ struct fd *fd; /* optional file descriptor to poll */ - unsigned int wake_bits; /* wakeup bits */ - unsigned int changed_bits; /* changed wakeup bits */ int paint_count; /* pending paint messages count */ int hotkey_count; /* pending hotkey messages count */ int quit_message; /* is there a pending quit message? */ @@ -300,8 +298,6 @@ static struct msg_queue *create_msg_queue( struct thread *thread, struct thread_ if ((queue = alloc_object( &msg_queue_ops ))) { queue->fd = NULL; - queue->wake_bits = 0; - queue->changed_bits = 0; queue->paint_count = 0; queue->hotkey_count = 0; queue->quit_message = 0; @@ -329,7 +325,9 @@ static struct msg_queue *create_msg_queue( struct thread *thread, struct thread_ { memset( (void *)shared->hooks_count, 0, sizeof(shared->hooks_count) ); shared->wake_mask = 0; + shared->wake_bits = 0; shared->changed_mask = 0; + shared->changed_bits = 0; } SHARED_WRITE_END; @@ -641,29 +639,44 @@ void add_queue_hook_count( struct thread *thread, unsigned int index, int count static inline int is_signaled( struct msg_queue *queue ) { const queue_shm_t *queue_shm = queue->shared; - return (queue->wake_bits & queue_shm->wake_mask) || - (queue->changed_bits & queue_shm->changed_mask); + return (queue_shm->wake_bits & queue_shm->wake_mask) || + (queue_shm->changed_bits & queue_shm->changed_mask); } /* set some queue bits */ static inline void set_queue_bits( struct msg_queue *queue, unsigned int bits ) { + const queue_shm_t *queue_shm = queue->shared; + if (bits & (QS_KEY | QS_MOUSEBUTTON)) { if (!queue->keystate_lock) lock_input_keystate( queue->input ); queue->keystate_lock = 1; } - queue->wake_bits |= bits; - queue->changed_bits |= bits; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_bits |= bits; + shared->changed_bits |= bits; + } + SHARED_WRITE_END; + if (is_signaled( queue )) wake_up( &queue->obj, 0 ); } /* clear some queue bits */ static inline void clear_queue_bits( struct msg_queue *queue, unsigned int bits ) { - queue->wake_bits &= ~bits; - queue->changed_bits &= ~bits; - if (!(queue->wake_bits & (QS_KEY | QS_MOUSEBUTTON))) + const queue_shm_t *queue_shm = queue->shared; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->wake_bits &= ~bits; + shared->changed_bits &= ~bits; + } + SHARED_WRITE_END; + + if (!(queue_shm->wake_bits & (QS_KEY | QS_MOUSEBUTTON))) { if (queue->keystate_lock) unlock_input_keystate( queue->input ); queue->keystate_lock = 0; @@ -1181,7 +1194,7 @@ static void msg_queue_dump( struct object *obj, int verbose ) struct msg_queue *queue = (struct msg_queue *)obj; const queue_shm_t *queue_shm = queue->shared; fprintf( stderr, "Msg queue bits=%x mask=%x\n", - queue->wake_bits, queue_shm->wake_mask ); + queue_shm->wake_bits, queue_shm->wake_mask ); } static int msg_queue_signaled( struct object *obj, struct wait_queue_entry *entry ) @@ -2955,8 +2968,9 @@ DECL_HANDLER(set_queue_mask) } SHARED_WRITE_END; - reply->wake_bits = queue->wake_bits; - reply->changed_bits = queue->changed_bits; + reply->wake_bits = queue_shm->wake_bits; + reply->changed_bits = queue_shm->changed_bits; + if (is_signaled( queue )) { /* if skip wait is set, do what would have been done in the subsequent wait */ @@ -2981,9 +2995,16 @@ DECL_HANDLER(get_queue_status) struct msg_queue *queue = current->queue; if (queue) { - reply->wake_bits = queue->wake_bits; - reply->changed_bits = queue->changed_bits; - queue->changed_bits &= ~req->clear_bits; + const queue_shm_t *queue_shm = queue->shared; + + reply->wake_bits = queue_shm->wake_bits; + reply->changed_bits = queue_shm->changed_bits; + + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) + { + shared->changed_bits &= ~req->clear_bits; + } + SHARED_WRITE_END; } else reply->wake_bits = reply->changed_bits = 0; } @@ -3167,13 +3188,17 @@ DECL_HANDLER(get_message) } /* clear changed bits so we can wait on them if we don't find a message */ - if (filter & QS_POSTMESSAGE) + SHARED_WRITE_BEGIN( queue_shm, queue_shm_t ) { - queue->changed_bits &= ~(QS_POSTMESSAGE | QS_HOTKEY | QS_TIMER); - if (req->get_first == 0 && req->get_last == ~0U) queue->changed_bits &= ~QS_ALLPOSTMESSAGE; + if (filter & QS_POSTMESSAGE) + { + shared->changed_bits &= ~(QS_POSTMESSAGE | QS_HOTKEY | QS_TIMER); + if (req->get_first == 0 && req->get_last == ~0U) shared->changed_bits &= ~QS_ALLPOSTMESSAGE; + } + if (filter & QS_INPUT) shared->changed_bits &= ~QS_INPUT; + if (filter & QS_PAINT) shared->changed_bits &= ~QS_PAINT; } - if (filter & QS_INPUT) queue->changed_bits &= ~QS_INPUT; - if (filter & QS_PAINT) queue->changed_bits &= ~QS_PAINT; + SHARED_WRITE_END; /* then check for posted messages */ if ((filter & QS_POSTMESSAGE) && -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/input.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index a582826d18e..999cce23715 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -841,6 +841,26 @@ SHORT WINAPI NtUserGetAsyncKeyState( INT key ) return ret; } +/*********************************************************************** + * get_shared_queue_bits + */ +static BOOL get_shared_queue_bits( UINT *wake_bits, UINT *changed_bits ) +{ + struct object_lock lock = OBJECT_LOCK_INIT; + const queue_shm_t *queue_shm; + UINT status; + + *wake_bits = *changed_bits = 0; + while ((status = get_shared_queue( &lock, &queue_shm )) == STATUS_PENDING) + { + *wake_bits = queue_shm->wake_bits; + *changed_bits = queue_shm->changed_bits; + } + + if (status) return FALSE; + return TRUE; +} + /*********************************************************************** * NtUserGetQueueStatus (win32u.@) */ @@ -871,18 +891,12 @@ DWORD WINAPI NtUserGetQueueStatus( UINT flags ) */ DWORD get_input_state(void) { - DWORD ret; + UINT wake_bits, changed_bits; check_for_events( QS_INPUT ); - SERVER_START_REQ( get_queue_status ) - { - req->clear_bits = 0; - wine_server_call( req ); - ret = reply->wake_bits & (QS_KEY | QS_MOUSEBUTTON); - } - SERVER_END_REQ; - return ret; + if (!get_shared_queue_bits( &wake_bits, &changed_bits )) return 0; + return wake_bits & (QS_KEY | QS_MOUSEBUTTON); } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/input.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dlls/win32u/input.c b/dlls/win32u/input.c index 999cce23715..8356e97e238 100644 --- a/dlls/win32u/input.c +++ b/dlls/win32u/input.c @@ -866,7 +866,7 @@ static BOOL get_shared_queue_bits( UINT *wake_bits, UINT *changed_bits ) */ DWORD WINAPI NtUserGetQueueStatus( UINT flags ) { - DWORD ret; + UINT ret, wake_bits, changed_bits; if (flags & ~(QS_ALLINPUT | QS_ALLPOSTMESSAGE | QS_SMRESULT)) { @@ -876,7 +876,9 @@ DWORD WINAPI NtUserGetQueueStatus( UINT flags ) check_for_events( flags ); - SERVER_START_REQ( get_queue_status ) + if (get_shared_queue_bits( &wake_bits, &changed_bits ) && !(changed_bits & flags)) + ret = MAKELONG( changed_bits & flags, wake_bits & flags ); + else SERVER_START_REQ( get_queue_status ) { req->clear_bits = flags; wine_server_call( req ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/win32u/message.c | 44 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index d8cb6f395b2..2f5ffc3a538 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -2643,6 +2643,40 @@ static BOOL process_hardware_message( MSG *msg, UINT hw_id, const struct hardwar return ret; } +/*********************************************************************** + * check_queue_bits + * + * returns TRUE and the queue wake bits and changed bits if we can skip a server request + * returns FALSE if we need to make a server request to update the queue masks or bits + */ +static BOOL check_queue_bits( UINT wake_mask, UINT changed_mask, UINT signal_bits, UINT clear_bits, + UINT *wake_bits, UINT *changed_bits ) +{ + struct object_lock lock = OBJECT_LOCK_INIT; + const queue_shm_t *queue_shm; + BOOL skip = FALSE; + UINT status; + + while ((status = get_shared_queue( &lock, &queue_shm )) == STATUS_PENDING) + { + /* if the masks need an update */ + if (queue_shm->wake_mask != wake_mask) skip = FALSE; + else if (queue_shm->changed_mask != changed_mask) skip = FALSE; + /* or if some bits need to be cleared, or queue is signaled */ + else if (queue_shm->wake_bits & signal_bits) skip = FALSE; + else if (queue_shm->changed_bits & clear_bits) skip = FALSE; + else + { + *wake_bits = queue_shm->wake_bits; + *changed_bits = queue_shm->changed_bits; + skip = TRUE; + } + } + + if (status) return FALSE; + return skip; +} + /*********************************************************************** * peek_message * @@ -3326,14 +3360,18 @@ static void wait_message_reply( UINT flags ) for (;;) { - unsigned int wake_bits = 0; + UINT wake_bits, changed_bits; - SERVER_START_REQ( set_queue_mask ) + if (check_queue_bits( wake_mask, wake_mask, wake_mask, wake_mask, + &wake_bits, &changed_bits )) + wake_bits = wake_bits & wake_mask; + else SERVER_START_REQ( set_queue_mask ) { req->wake_mask = wake_mask; req->changed_mask = wake_mask; req->skip_wait = 1; - if (!wine_server_call( req )) wake_bits = reply->wake_bits & wake_mask; + wine_server_call( req ); + wake_bits = reply->wake_bits & wake_mask; } SERVER_END_REQ; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5940
Fwiw, check_queue_bits uses separate parameters, yet is called with the same mask in wait_message_reply, because it will also later be used in peek_message which needs the separate masks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5940#note_74522
LGTM. I checked it over several days and on several games, there are no problems with ntsync patches. Therefore the problem with wake_bits, changed_bits and ntsync was not confirmed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5940#note_74796
Thanks a lot for the testing. I think the previous version of the branch was bogus and I've redone the message queue patches since then. There was also some issues with the thread input shared memory in upcoming patches which I've fixed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5940#note_74798
participants (2)
-
Grigory Vasilyev (＠h0tc0d3) -
Rémi Bernon