[PATCH v2 0/3] MR8985: server: Do not clear queue masks in msg_queue_satified.
The masks are checked in win32u, and updated if necessary, right before waiting for messages. They tell wineserver which of the bits we want the queue to be signaled for in the following wait. There should be no need to clear the mask after waiting, and doing it will always force clients to update them again on the next wait, causing an additional request. -- v2: server: Remove skip_wait flag from set_queue_mask. server: Do not clear queue masks in msg_queue_satified. server: Rename queue is_signaled to get_queue_status. https://gitlab.winehq.org/wine/wine/-/merge_requests/8985
From: Rémi Bernon <rbernon(a)codeweavers.com> --- server/queue.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/queue.c b/server/queue.c index 596742bc64c..0e0339109fa 100644 --- a/server/queue.c +++ b/server/queue.c @@ -729,7 +729,7 @@ static void reset_queue_sync( struct msg_queue *queue ) } /* check the queue status */ -static inline int is_signaled( struct msg_queue *queue ) +static inline int get_queue_status( struct msg_queue *queue ) { queue_shm_t *queue_shm = queue->shared; return (queue_shm->wake_bits & queue_shm->wake_mask) || @@ -754,7 +754,7 @@ static inline void set_queue_bits( struct msg_queue *queue, unsigned int bits ) } SHARED_WRITE_END; - if (is_signaled( queue )) signal_queue_sync( queue ); + if (get_queue_status( queue )) signal_queue_sync( queue ); } /* clear some queue bits */ @@ -774,7 +774,7 @@ static inline void clear_queue_bits( struct msg_queue *queue, unsigned int bits if (queue->keystate_lock) unlock_input_keystate( queue->input ); queue->keystate_lock = 0; } - if (!is_signaled( queue )) reset_queue_sync( queue ); + if (!get_queue_status( queue )) reset_queue_sync( queue ); } /* check if message is matched by the filter */ @@ -3178,7 +3178,7 @@ DECL_HANDLER(set_queue_mask) reply->wake_bits = queue_shm->wake_bits; reply->changed_bits = queue_shm->changed_bits; - if (!is_signaled( queue )) reset_queue_sync( queue ); + if (!get_queue_status( queue )) reset_queue_sync( queue ); else if (!req->skip_wait) signal_queue_sync( queue ); else msg_queue_satisfied( &queue->obj, NULL ); } @@ -3202,7 +3202,7 @@ DECL_HANDLER(get_queue_status) } SHARED_WRITE_END; - if (!is_signaled( queue )) reset_queue_sync( queue ); + if (!get_queue_status( queue )) reset_queue_sync( queue ); } else reply->wake_bits = reply->changed_bits = 0; } @@ -3400,7 +3400,7 @@ DECL_HANDLER(get_message) } SHARED_WRITE_END; - if (!is_signaled( queue )) reset_queue_sync( queue ); + if (!get_queue_status( queue )) reset_queue_sync( queue ); /* then check for posted messages */ if ((filter & QS_POSTMESSAGE) && -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8985
From: Rémi Bernon <rbernon(a)codeweavers.com> The masks are checked in win32u, and updated if necessary, right before waiting for messages. They tell wineserver which of the bits we want the queue to be signaled for in the following wait. There should be no need to clear the mask after waiting, and doing it will always force clients to update them again on the next wait, causing an additional request. --- server/queue.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/server/queue.c b/server/queue.c index 0e0339109fa..9994e0fbe6c 100644 --- a/server/queue.c +++ b/server/queue.c @@ -1348,14 +1348,6 @@ 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_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; reset_queue_sync( queue ); } @@ -3180,7 +3172,7 @@ DECL_HANDLER(set_queue_mask) if (!get_queue_status( queue )) reset_queue_sync( queue ); else if (!req->skip_wait) signal_queue_sync( queue ); - else msg_queue_satisfied( &queue->obj, NULL ); + else reset_queue_sync( queue ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8985
From: Rémi Bernon <rbernon(a)codeweavers.com> Resetting the queue sync doesn't look right or necessary anymore. The sync state should reflect the bits vs mask status at all time and the sync will be reset whenever masks are updated, for instance before any wait on the queue. --- dlls/win32u/message.c | 2 -- server/protocol.def | 1 - server/queue.c | 3 +-- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 035ab6574e4..3a3f79a2ee8 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -3219,7 +3219,6 @@ static DWORD wait_objects( DWORD count, const HANDLE *handles, DWORD timeout, { req->wake_mask = wake_mask; req->changed_mask = changed_mask; - req->skip_wait = 0; wine_server_call( req ); } SERVER_END_REQ; @@ -3540,7 +3539,6 @@ static void wait_message_reply( UINT flags ) { req->wake_mask = wake_mask; req->changed_mask = wake_mask; - req->skip_wait = 1; wine_server_call( req ); wake_bits = reply->wake_bits & wake_mask; } diff --git a/server/protocol.def b/server/protocol.def index 6bb7ae155c6..4237cb53307 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2286,7 +2286,6 @@ struct process_info @REQ(set_queue_mask) unsigned int wake_mask; /* wakeup bits mask */ unsigned int changed_mask; /* changed bits mask */ - int skip_wait; /* will we skip waiting if signaled? */ @REPLY unsigned int wake_bits; /* current wake bits */ unsigned int changed_bits; /* current changed bits */ diff --git a/server/queue.c b/server/queue.c index 9994e0fbe6c..40f534f7bb2 100644 --- a/server/queue.c +++ b/server/queue.c @@ -3171,8 +3171,7 @@ DECL_HANDLER(set_queue_mask) reply->changed_bits = queue_shm->changed_bits; if (!get_queue_status( queue )) reset_queue_sync( queue ); - else if (!req->skip_wait) signal_queue_sync( queue ); - else reset_queue_sync( queue ); + else signal_queue_sync( queue ); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8985
participants (1)
-
Rémi Bernon