[PATCH 0/1] MR9809: server: Support creating files with direct and synchronous I/O
This allows various disk benchmarking software like CrystalDiskMark and ATTO disk benchmark to actually benchmark the disk rather than the file cache Implementation based on public Microsoft NtCreateFile() documentation: https://learn.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntc... According to the documentation, Windows forbids FILE_APPEND_DATA and FILE_NO_INTERMEDIATE_BUFFERING being set together, but Linux has no such limitation with the equivalent O_APPEND and O_DIRECT... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9809
From: 小太 <nospam@kota.moe> This allows various disk benchmarking software like CrystalDiskMark and ATTO disk benchmark to actually benchmark the disk rather than the file cache Implementation based on public Microsoft NtCreateFile() documentation: https://learn.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntc... According to the documentation, Windows forbids FILE_APPEND_DATA and FILE_NO_INTERMEDIATE_BUFFERING being set together, but Linux has no such limitation with the equivalent O_APPEND and O_DIRECT... --- server/fd.c | 8 +++++++- server/file.c | 9 +++++++++ server/unicode.c | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/server/fd.c b/server/fd.c index f70bec354a3..ea11107bd1c 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1927,7 +1927,9 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam char *path; if (((options & FILE_DELETE_ON_CLOSE) && !(access & DELETE)) || - ((options & FILE_DIRECTORY_FILE) && (flags & O_TRUNC))) + ((options & FILE_DIRECTORY_FILE) && (flags & O_TRUNC)) || + ((options & FILE_SYNCHRONOUS_IO_ALERT) && !(access & SYNCHRONIZE)) || + ((options & FILE_SYNCHRONOUS_IO_NONALERT) && !(access & SYNCHRONIZE))) { set_error( STATUS_INVALID_PARAMETER ); return NULL; @@ -1974,6 +1976,10 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam } else rw_mode = O_RDONLY; + if (options & FILE_NO_INTERMEDIATE_BUFFERING) flags |= O_DIRECT; + if (options & FILE_SYNCHRONOUS_IO_NONALERT) flags |= O_SYNC; + if (options & FILE_SYNCHRONOUS_IO_ALERT) flags |= O_SYNC; + if ((fd->unix_fd = open( name, rw_mode | (flags & ~O_TRUNC), *mode )) == -1) { /* if we tried to open a directory for write access, retry read-only */ diff --git a/server/file.c b/server/file.c index cc5acc2aadc..3f205bb7a8e 100644 --- a/server/file.c +++ b/server/file.c @@ -220,6 +220,15 @@ static struct object *create_file( struct fd *root, const char *nameptr, data_si char *name; mode_t mode; + if (((options & FILE_NO_INTERMEDIATE_BUFFERING) && (access & FILE_APPEND_DATA))) + { + /* map_access() below will set FILE_APPEND_DATA for any WRITE access, so + checking for this Windows-only error condition has to be done here + instead of in open_fd() */ + set_error( STATUS_INVALID_PARAMETER ); + return NULL; + } + if (!len || ((nameptr[0] == '/') ^ !root) || (nt_name.len && ((nt_name.str[0] == '\\') ^ !root))) { set_error( STATUS_OBJECT_PATH_SYNTAX_BAD ); diff --git a/server/unicode.c b/server/unicode.c index bb39b55e50c..630db4cfff8 100644 --- a/server/unicode.c +++ b/server/unicode.c @@ -343,7 +343,7 @@ struct fd *load_intl_file(void) { if (!nls_dirs[i]) continue; if (asprintf( &path, "%s/l_intl.nls", nls_dirs[i] ) == -1) continue; - if ((fd = open_fd( NULL, path, nt_name, O_RDONLY, &mode, FILE_READ_DATA, + if ((fd = open_fd( NULL, path, nt_name, O_RDONLY, &mode, FILE_READ_DATA | SYNCHRONIZE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT ))) break; free( path ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9809
Looks like the parameter validation I added (based on the documentation) breaks a bunch of tests in obscure places - presumably because they were using the API incorrectly. Do you think it's worth going through and fixing all usages, or should I just remove the validation? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9809#note_125971
On Sun Dec 21 17:30:37 2025 +0000, 小太 wrote:
Looks like the parameter validation I added (based on the documentation) breaks a bunch of tests in obscure places - presumably because they were using the API incorrectly. Do you think it's worth going through and fixing all usages, or should I just remove the validation? Many poorly-written Windows apps and games "use the API incorrectly." We have to support them, so our tests *intentionally* emulate the incorrect API usage.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9809#note_125972
Note that Wine has differences with Windows WRT file IO optimization details (for one, we don’t support asynchronous IO for regular files and perform it synchronously). IO caching works differently on Windows and Linux, I think Linux normally caches much more. I am afraid that making no buffering flag really work may do more harm than good by affecting apps IO perfectly if they use it. I’d suggest not to try to actually favour non-buffered IO flag motivated only by benchmarking software. Also, O_DIRECT implies limitations (e.g. extra alignment for data), not sure this can work as is. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9809#note_125973
Looks like the parameter validation I added (based on the documentation) breaks a bunch of tests in obscure places - presumably because they were using the API incorrectly.
Do you think it's worth going through and fixing all usages, or should I just remove the validation?
If the tests are breaking, and they currently pass on Windows, presumably the validation is wrong. Microsoft's documentation is not always truthful. Any validation like this that's user-visible should have tests anyway. ``` + if (options & FILE_SYNCHRONOUS_IO_NONALERT) flags |= O_SYNC; + if (options & FILE_SYNCHRONOUS_IO_ALERT) flags |= O_SYNC; ``` That's not the same semantics. FILE_SYNCHRONOUS_IO_* is about whether the I/O operation will return STATUS_SUCCESS or STATUS_PENDING (and we already implement it). It doesn't guarantee that the data will be written to disk. I think the corresponding flag there is actually FILE_WRITE_THROUGH. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9809#note_125974
participants (5)
-
Elizabeth Figura (＠zfigura) -
Jinoh Kang (＠iamahuman)
-
Paul Gofman (＠gofman)
-
小太
-
小太 (＠kotarou3)