[PATCH 0/2] MR5862: server: Take advantage of server-side ioctl handling to simplify IOCTL_SERIAL_WAIT_ON_MASK.

List overview All Threads

newer

older

[PATCH 0/3] MR5863: odbc32: Handle...

Re: [PATCH v3 0/5] MR5794: Cmd...

Elizabeth Figura (＠zfigura)

17 Jun 2024 17 Jun '24

10:04 p.m.

Tested with a loopback serial pair.

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5862

Show replies by date

Elizabeth Figura

17 Jun 17 Jun

10:04 p.m.

New subject: [PATCH 1/2] server: Check for an existing serial wait ioctl within the ioctl handler.

From: Elizabeth Figura zfigura@codeweavers.com

--- dlls/ntdll/unix/serial.c | 22 +++------------------- include/wine/server_protocol.h | 3 +-- server/protocol.def | 1 - server/serial.c | 31 ++++++------------------------- 4 files changed, 10 insertions(+), 47 deletions(-)

diff --git a/dlls/ntdll/unix/serial.c b/dlls/ntdll/unix/serial.c index d73ece47d0b..5af3f9afced 100644 --- a/dlls/ntdll/unix/serial.c +++ b/dlls/ntdll/unix/serial.c @@ -452,21 +452,7 @@ static NTSTATUS get_status(int fd, SERIAL_STATUS* ss) return status; }

-static void stop_waiting( HANDLE handle ) -{ - unsigned int status; - - SERVER_START_REQ( set_serial_info ) - { - req->handle = wine_server_obj_handle( handle ); - req->flags = SERIALINFO_PENDING_WAIT; - if ((status = wine_server_call( req ))) - ERR("failed to clear waiting state: %#x\n", status); - } - SERVER_END_REQ; -} - -static NTSTATUS get_wait_mask(HANDLE hDevice, UINT *mask, UINT *cookie, BOOL *pending_write, BOOL start_wait) +static NTSTATUS get_wait_mask( HANDLE hDevice, UINT *mask, UINT *cookie, BOOL *pending_write ) { unsigned int status;

@@ -474,7 +460,6 @@ static NTSTATUS get_wait_mask(HANDLE hDevice, UINT *mask, UINT *cookie, BOOL *pe { req->handle = wine_server_obj_handle( hDevice ); req->flags = pending_write ? SERIALINFO_PENDING_WRITE : 0; - if (start_wait) req->flags |= SERIALINFO_PENDING_WAIT; if (!(status = wine_server_call( req ))) { *mask = reply->eventmask; @@ -1126,7 +1111,7 @@ static BOOL async_wait_proc( void *user, ULONG_PTR *info, unsigned int *status ) } else { - get_wait_mask( commio->io.handle, &dummy, &cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL, FALSE ); + get_wait_mask( commio->io.handle, &dummy, &cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); if (commio->cookie != cookie) { *commio->events = 0; @@ -1143,7 +1128,6 @@ static BOOL async_wait_proc( void *user, ULONG_PTR *info, unsigned int *status )

if (needs_close) close( fd ); } - stop_waiting( commio->io.handle ); release_fileio( &commio->io ); return TRUE; } @@ -1161,7 +1145,7 @@ static NTSTATUS wait_on( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE ap

commio->events = out_buffer; commio->pending_write = 0; - status = get_wait_mask( handle, &commio->evtmask, &commio->cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL, TRUE ); + status = get_wait_mask( handle, &commio->evtmask, &commio->cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); if (status) { free( commio ); diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 8a5ae71b856..71e28f8eac2 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -3035,7 +3035,6 @@ struct set_serial_info_reply struct reply_header __header; }; #define SERIALINFO_PENDING_WRITE 0x04 -#define SERIALINFO_PENDING_WAIT 0x08

struct cancel_sync_request @@ -6534,7 +6533,7 @@ union generic_reply

/* ### protocol_version begin ### */

-#define SERVER_PROTOCOL_VERSION 806 +#define SERVER_PROTOCOL_VERSION 807

/* ### protocol_version end ### */

diff --git a/server/protocol.def b/server/protocol.def index 25184641082..b543fdabaae 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2256,7 +2256,6 @@ enum message_type int flags; /* bitmask to set values (see below) */ @END #define SERIALINFO_PENDING_WRITE 0x04 -#define SERIALINFO_PENDING_WAIT 0x08

/* Cancel all sync io on a thread */ @REQ(cancel_sync) diff --git a/server/serial.c b/server/serial.c index 68b489822b4..2fab9bbe7e6 100644 --- a/server/serial.c +++ b/server/serial.c @@ -78,7 +78,6 @@ struct serial unsigned int eventmask; unsigned int generation; /* event mask change counter */ unsigned int pending_write : 1; - unsigned int pending_wait : 1;

struct termios original;

@@ -144,7 +143,6 @@ struct object *create_serial( struct fd *fd ) serial->eventmask = 0; serial->generation = 0; serial->pending_write = 0; - serial->pending_wait = 0; memset( &serial->timeouts, 0, sizeof(serial->timeouts) ); init_async_queue( &serial->wait_q ); serial->fd = (struct fd *)grab_object( fd ); @@ -250,6 +248,12 @@ static void serial_ioctl( struct fd *fd, ioctl_code_t code, struct async *async { struct wait_req *req;

+ if (async_queued( &serial->wait_q )) + { + set_error( STATUS_INVALID_PARAMETER ); + return; + } + if (!(req = mem_alloc(sizeof(*req)))) return;

@@ -329,17 +333,6 @@ DECL_HANDLER(get_serial_info)

if ((serial = get_serial_obj( current->process, req->handle, 0 ))) { - if (req->flags & SERIALINFO_PENDING_WAIT) - { - if (serial->pending_wait) - { - release_object( serial ); - set_error( STATUS_INVALID_PARAMETER ); - return; - } - serial->pending_wait = 1; - } - /* event mask */ reply->eventmask = serial->eventmask; reply->cookie = serial->generation; @@ -359,18 +352,6 @@ DECL_HANDLER(set_serial_info)

if ((serial = get_serial_obj( current->process, req->handle, 0 ))) { - if (req->flags & SERIALINFO_PENDING_WAIT) - { - if (!serial->pending_wait) - { - release_object( serial ); - set_error( STATUS_INVALID_PARAMETER ); - return; - } - serial->pending_wait = 0; - } - - /* pending write */ if (req->flags & SERIALINFO_PENDING_WRITE) serial->pending_write = 1;

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5862

Elizabeth Figura

10:04 p.m.

New subject: [PATCH 2/2] server: Directly wake up wait asyncs when the serial mask changes.

From: Elizabeth Figura zfigura@codeweavers.com

diff --git a/dlls/ntdll/unix/serial.c b/dlls/ntdll/unix/serial.c index 5af3f9afced..ea9931d891a 100644 --- a/dlls/ntdll/unix/serial.c +++ b/dlls/ntdll/unix/serial.c @@ -452,7 +452,7 @@ static NTSTATUS get_status(int fd, SERIAL_STATUS* ss) return status; }

-static NTSTATUS get_wait_mask( HANDLE hDevice, UINT *mask, UINT *cookie, BOOL *pending_write ) +static NTSTATUS get_wait_mask( HANDLE hDevice, UINT *mask, BOOL *pending_write ) { unsigned int status;

@@ -463,7 +463,6 @@ static NTSTATUS get_wait_mask( HANDLE hDevice, UINT *mask, UINT *cookie, BOOL *p if (!(status = wine_server_call( req ))) { *mask = reply->eventmask; - if (cookie) *cookie = reply->cookie; if (pending_write) *pending_write = reply->pending_write; } } @@ -967,7 +966,6 @@ typedef struct async_commio struct async_fileio io; DWORD* events; UINT evtmask; - UINT cookie; UINT mstat; BOOL pending_write; serial_irq_info irq_info; @@ -1080,7 +1078,7 @@ static BOOL async_wait_proc( void *user, ULONG_PTR *info, unsigned int *status ) if (!server_get_unix_fd( commio->io.handle, FILE_READ_DATA | FILE_WRITE_DATA, &fd, &needs_close, NULL, NULL )) { serial_irq_info new_irq_info; - UINT new_mstat, dummy, cookie; + UINT new_mstat, dummy;

TRACE( "device=%p fd=0x%08x mask=0x%08x buffer=%p irq_info=%p\n", commio->io.handle, fd, commio->evtmask, commio->events, &commio->irq_info ); @@ -1111,18 +1109,9 @@ static BOOL async_wait_proc( void *user, ULONG_PTR *info, unsigned int *status ) } else { - get_wait_mask( commio->io.handle, &dummy, &cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); - if (commio->cookie != cookie) - { - *commio->events = 0; - *status = STATUS_CANCELLED; - *info = 0; - } - else - { - if (needs_close) close( fd ); - return FALSE; - } + get_wait_mask( commio->io.handle, &dummy, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); + if (needs_close) close( fd ); + return FALSE; } }

@@ -1145,7 +1134,7 @@ static NTSTATUS wait_on( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE ap

commio->events = out_buffer; commio->pending_write = 0; - status = get_wait_mask( handle, &commio->evtmask, &commio->cookie, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); + status = get_wait_mask( handle, &commio->evtmask, (commio->evtmask & EV_TXEMPTY) ? &commio->pending_write : NULL ); if (status) { free( commio ); diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 71e28f8eac2..e16f1ceb7c3 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -3016,9 +3016,7 @@ struct get_serial_info_reply { struct reply_header __header; unsigned int eventmask; - unsigned int cookie; unsigned int pending_write; - char __pad_20[4]; };

@@ -6533,7 +6531,7 @@ union generic_reply

/* ### protocol_version begin ### */

-#define SERVER_PROTOCOL_VERSION 807 +#define SERVER_PROTOCOL_VERSION 808

/* ### protocol_version end ### */

diff --git a/server/protocol.def b/server/protocol.def index b543fdabaae..766674d3e30 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -2245,7 +2245,6 @@ enum message_type int flags; @REPLY unsigned int eventmask; - unsigned int cookie; unsigned int pending_write; @END

diff --git a/server/request.h b/server/request.h index acee48c2faf..67de9bb1779 100644 --- a/server/request.h +++ b/server/request.h @@ -1455,9 +1455,8 @@ C_ASSERT( FIELD_OFFSET(struct get_serial_info_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct get_serial_info_request, flags) == 16 ); C_ASSERT( sizeof(struct get_serial_info_request) == 24 ); C_ASSERT( FIELD_OFFSET(struct get_serial_info_reply, eventmask) == 8 ); -C_ASSERT( FIELD_OFFSET(struct get_serial_info_reply, cookie) == 12 ); -C_ASSERT( FIELD_OFFSET(struct get_serial_info_reply, pending_write) == 16 ); -C_ASSERT( sizeof(struct get_serial_info_reply) == 24 ); +C_ASSERT( FIELD_OFFSET(struct get_serial_info_reply, pending_write) == 12 ); +C_ASSERT( sizeof(struct get_serial_info_reply) == 16 ); C_ASSERT( FIELD_OFFSET(struct set_serial_info_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct set_serial_info_request, flags) == 16 ); C_ASSERT( sizeof(struct set_serial_info_request) == 24 ); diff --git a/server/serial.c b/server/serial.c index 2fab9bbe7e6..209f2e9174e 100644 --- a/server/serial.c +++ b/server/serial.c @@ -76,7 +76,6 @@ struct serial struct timeout_user *read_timer; SERIAL_TIMEOUTS timeouts; unsigned int eventmask; - unsigned int generation; /* event mask change counter */ unsigned int pending_write : 1;

struct termios original; @@ -141,7 +140,6 @@ struct object *create_serial( struct fd *fd )

serial->read_timer = NULL; serial->eventmask = 0; - serial->generation = 0; serial->pending_write = 0; memset( &serial->timeouts, 0, sizeof(serial->timeouts) ); init_async_queue( &serial->wait_q ); @@ -240,8 +238,7 @@ static void serial_ioctl( struct fd *fd, ioctl_code_t code, struct async *async return; } serial->eventmask = *(unsigned int *)get_req_data(); - serial->generation++; - fd_async_wake_up( serial->fd, ASYNC_TYPE_WAIT, STATUS_SUCCESS ); + async_wake_up( &serial->wait_q, STATUS_CANCELLED ); return;

case IOCTL_SERIAL_WAIT_ON_MASK: @@ -335,7 +332,6 @@ DECL_HANDLER(get_serial_info) { /* event mask */ reply->eventmask = serial->eventmask; - reply->cookie = serial->generation;

/* pending write */ reply->pending_write = serial->pending_write; diff --git a/server/trace.c b/server/trace.c index 1aa0d5c1518..08adda28230 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2856,7 +2856,6 @@ static void dump_get_serial_info_request( const struct get_serial_info_request * static void dump_get_serial_info_reply( const struct get_serial_info_reply *req ) { fprintf( stderr, " eventmask=%08x", req->eventmask ); - fprintf( stderr, ", cookie=%08x", req->cookie ); fprintf( stderr, ", pending_write=%08x", req->pending_write ); }

-- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5862

Marvin

10:38 p.m.

New subject: [PATCH 2/2] server: Directly wake up wait asyncs when the serial mask changes.

Hi,

It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=146341

Your paranoid android.

=== debian11b (64 bit WoW report) ===

user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000009000E2, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got hook 0x0005 instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got winevent_hook 0x0003 instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x030f instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x001c instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x0086 instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got msg 0x0006 instead msg.c:6933: Test failed: SetFocus(hwnd) on a button: 3: the winevent_hook 0x8005 was expected, but got hook 0x0009 instead

233

Age (days ago)

233

Last active (days ago)

wine-gitlab@winehq.org

3 comments

3 participants

tags (0)

participants (3)

Elizabeth Figura
Elizabeth Figura (＠zfigura)
Marvin