[PATCH v4 1/4] msvcrt/tests: Add test for fopen/_open access hints
Test passing sequential and random access hints to fopen and _open. Test that the underlying windows handle actually has the corresponding FILE_SEQUENTIAL_ONLY mode set correctly. Signed-off-by: Luke Deller <luke(a)deller.id.au> --- v4: Adjust the test for the contradictory "rbRS" fopen mode, to match what Marvin <testbot(a)winehq.org> reported is the behaviour on Windows --- dlls/msvcrt/tests/file.c | 124 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c index fb242d81282..1117ac0f729 100644 --- a/dlls/msvcrt/tests/file.c +++ b/dlls/msvcrt/tests/file.c @@ -35,6 +35,7 @@ #include <process.h> #include <errno.h> #include <locale.h> +#include <winternl.h> #define MSVCRT_FD_BLOCK_SIZE 32 typedef struct { @@ -2731,6 +2732,127 @@ static void test_lseek(void) DeleteFileA("_creat.tst"); } +static BOOL has_sequential_hint(int fd) +{ + HANDLE handle; + FILE_MODE_INFORMATION mode_info; + IO_STATUS_BLOCK io; + NTSTATUS status; + + /* Check whether the file handle has the sequential access hint */ + handle = (HANDLE)_get_osfhandle(fd); + status = NtQueryInformationFile(handle, &io, &mode_info, sizeof(mode_info), + FileModeInformation); + ok(!status, "NtQueryInformationFile failed\n"); + return (mode_info.Mode & FILE_SEQUENTIAL_ONLY) != 0; +} + +static void test_fopen_hints(void) +{ + char *tempf; + FILE *fp; + int i; + + /* fopen modes to test */ + const char *fopen_modes[] = { + "rb", "rbS", "rbR", "rbSR", "rbRS" + }; + /* corresponding expected hints */ + BOOL expected_hints[][2] = { + /* expect_sequential, expect_random */ + {FALSE, FALSE}, + {TRUE, FALSE}, + {FALSE, TRUE}, + {TRUE, FALSE}, + {FALSE, TRUE}, + }; + + /* create a test file */ + tempf = _tempnam(".","wne"); + fp = fopen(tempf, "wb"); + ok(fp != NULL, "unable to create test file\n"); + fwrite("abc\n", 1, 4, fp); + fclose(fp); + + /* test fopen with each mode */ + for (i = 0; i < sizeof(fopen_modes)/sizeof(char*); ++i) + { + BOOL expect_sequential, has_sequential; + const char *mode = fopen_modes[i]; + expect_sequential = expected_hints[i][0]; + + fp = fopen(tempf, mode); + ok(fp != NULL, "unable to fopen test file with mode \"%s\"\n", mode); + + has_sequential = has_sequential_hint(_fileno(fp)); + todo_wine_if(expect_sequential) ok(has_sequential == expect_sequential, + "unexpected sequential hint %d for fopen mode \"%s\"\n", + has_sequential, mode); + + /* TODO: Somehow check whether the random access hint has been applied. + * Sadly NtQueryInformationFile does not expose this information. */ + + fclose(fp); + } + + unlink(tempf); + free(tempf); +} + +static void test_open_hints(void) +{ + char *tempf; + int fd; + int i; + + /* fopen modes to test */ + int open_flags[] = { + _O_RDONLY | _O_BINARY, + _O_RDONLY | _O_BINARY | _O_SEQUENTIAL, + _O_RDONLY | _O_BINARY | _O_RANDOM, + _O_RDONLY | _O_BINARY | _O_RANDOM | _O_SEQUENTIAL, + }; + /* corresponding expected hints */ + BOOL expected_hints[][2] = { + /* expect_sequential, expect_random */ + {FALSE, FALSE}, + {TRUE, FALSE}, + {FALSE, TRUE}, + {TRUE, TRUE}, + }; + + /* create a test file */ + tempf = _tempnam(".","wne"); + fd = _open(tempf, _O_CREAT | _O_WRONLY | _O_BINARY); + ok(fd != -1, "unable to create test file\n"); + _write(fd, "abc\n", 4); + _close(fd); + + /* test _open with each mode */ + for (i = 0; i < sizeof(open_flags)/sizeof(int); ++i) + { + BOOL expect_sequential, has_sequential; + int flags = open_flags[i]; + expect_sequential = expected_hints[i][0]; + + fd = open(tempf, flags); + ok(fd != -1, "unable to _open test file with flags %x\n", flags); + + has_sequential = has_sequential_hint(fd); + todo_wine_if(expect_sequential) ok(has_sequential == expect_sequential, + "unexpected sequential hint %d for _open flags %x\n", + has_sequential, flags); + + /* TODO: Somehow check whether the random access hint has been applied. + * Sadly NtQueryInformationFile does not expose this information. */ + + close(fd); + } + + unlink(tempf); + free(tempf); +} + START_TEST(file) { int arg_c; @@ -2803,6 +2925,8 @@ START_TEST(file) test_close(); test__creat(); test_lseek(); + test_fopen_hints(); + test_open_hints(); /* Wait for the (_P_NOWAIT) spawned processes to finish to make sure the report * file contains lines in the correct order -- 2.25.1
Translate access pattern hints supplied to fopen / _open into the corresponding attributes for the lower level CreateFileW call. Signed-off-by: Luke Deller <luke(a)deller.id.au> --- v4: Adjust the behaviour of fopen when both 'S' (for sequential access) and 'R' (for random access) are supplied, so that only the first of these is used and the second ignored, to match what Marvin the testbot reported is the behaviour on real Windows. --- dlls/msvcrt/file.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c index 6e955099090..b0eeaf2a351 100644 --- a/dlls/msvcrt/file.c +++ b/dlls/msvcrt/file.c @@ -22,8 +22,6 @@ * License along with this library; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA * - * TODO - * Use the file flag hints O_SEQUENTIAL, O_RANDOM, O_SHORT_LIVED */ #include <direct.h> @@ -1609,8 +1607,12 @@ static int msvcrt_get_flags(const wchar_t* mode, int *open_flags, int* stream_fl case 'w': break; case 'S': + if (!(*open_flags & _O_RANDOM)) + *open_flags |= _O_SEQUENTIAL; + break; case 'R': - FIXME("ignoring cache optimization flag: %c\n", mode[-1]); + if (!(*open_flags & _O_SEQUENTIAL)) + *open_flags |= _O_RANDOM; break; default: ERR("incorrect mode flag: %c\n", mode[-1]); @@ -2269,6 +2271,13 @@ int CDECL _wsopen_dispatch( const wchar_t* path, int oflags, int shflags, int pm sharing |= FILE_SHARE_DELETE; } + if (oflags & _O_RANDOM) + attrib |= FILE_FLAG_RANDOM_ACCESS; + if (oflags & _O_SEQUENTIAL) + attrib |= FILE_FLAG_SEQUENTIAL_SCAN; + if (oflags & _O_SHORT_LIVED) + attrib |= FILE_ATTRIBUTE_TEMPORARY; + sa.nLength = sizeof( SECURITY_ATTRIBUTES ); sa.lpSecurityDescriptor = NULL; sa.bInheritHandle = !(oflags & _O_NOINHERIT); -- 2.25.1
Translate FILE_FLAG_SEQUENTIAL_SCAN into the corresponding flag for NtCreateFile options Signed-off-by: Luke Deller <luke(a)deller.id.au> --- dlls/kernelbase/file.c | 3 +++ dlls/msvcrt/tests/file.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index e0a75c2ad08..d21ad299d1f 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -723,6 +723,8 @@ static UINT get_nt_file_options( DWORD attributes ) options |= FILE_SYNCHRONOUS_IO_NONALERT; if (attributes & FILE_FLAG_RANDOM_ACCESS) options |= FILE_RANDOM_ACCESS; + if (attributes & FILE_FLAG_SEQUENTIAL_SCAN) + options |= FILE_SEQUENTIAL_ONLY; if (attributes & FILE_FLAG_WRITE_THROUGH) options |= FILE_WRITE_THROUGH; return options; @@ -3227,6 +3229,7 @@ HANDLE WINAPI DECLSPEC_HOTPATCH OpenFileById( HANDLE handle, LPFILE_ID_DESCRIPTO if (flags & FILE_FLAG_NO_BUFFERING) options |= FILE_NO_INTERMEDIATE_BUFFERING; if (!(flags & FILE_FLAG_OVERLAPPED)) options |= FILE_SYNCHRONOUS_IO_NONALERT; if (flags & FILE_FLAG_RANDOM_ACCESS) options |= FILE_RANDOM_ACCESS; + if (flags & FILE_FLAG_SEQUENTIAL_SCAN) options |= FILE_SEQUENTIAL_ONLY; flags &= FILE_ATTRIBUTE_VALID_FLAGS; objectName.Length = sizeof(ULONGLONG); diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c index 1117ac0f729..7a55e040d1c 100644 --- a/dlls/msvcrt/tests/file.c +++ b/dlls/msvcrt/tests/file.c @@ -2785,7 +2785,7 @@ static void test_fopen_hints(void) ok(fp != NULL, "unable to fopen test file with mode \"%s\"\n", mode); has_sequential = has_sequential_hint(_fileno(fp)); - todo_wine_if(expect_sequential) ok(has_sequential == expect_sequential, + ok(has_sequential == expect_sequential, "unexpected sequential hint %d for fopen mode \"%s\"\n", has_sequential, mode); @@ -2839,7 +2839,7 @@ static void test_open_hints(void) ok(fd != -1, "unable to _open test file with flags %x\n", flags); has_sequential = has_sequential_hint(fd); - todo_wine_if(expect_sequential) ok(has_sequential == expect_sequential, + ok(has_sequential == expect_sequential, "unexpected sequential hint %d for _open flags %x\n", has_sequential, flags); -- 2.25.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=94924 Your paranoid android. === w8 (32 bit report) === msvcrt: file.c:1688: Test failed: handle = 00000010 file.c:1692: Test failed: info->handle = 00000010 file.c:1695: Test failed: stdin->_file = 0 file.c:1700: Test failed: errno = 22 file.c:1710: Test failed: errno = 22 file.c:1730: Test failed: errno = 22 file.c:1734: Test failed: fclose(stdin) returned 0 file.c:1735: Test failed: errno = -559038737
Implement file access pattern hints for sequential or random file access using posix_fadvise if available. On Linux this will influence the kernel's readahead behaviour. Signed-off-by: Luke Deller <luke(a)deller.id.au> --- v3: if both flags are set (which is contradictory) then ignore them --- server/fd.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/fd.c b/server/fd.c index b953da2ab85..b20fa97229c 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1892,6 +1892,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam int root_fd = -1; int rw_mode; char *path; + unsigned int cache_hint; if (((options & FILE_DELETE_ON_CLOSE) && !(access & DELETE)) || ((options & FILE_DIRECTORY_FILE) && (flags & O_TRUNC))) @@ -2036,6 +2037,15 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam free( closed_fd ); fd->cacheable = 1; } + +#if _POSIX_C_SOURCE > 200112L + cache_hint = options & (FILE_SEQUENTIAL_ONLY | FILE_RANDOM_ACCESS); + if (cache_hint == FILE_SEQUENTIAL_ONLY) + posix_fadvise( fd->unix_fd, 0, 0, POSIX_FADV_SEQUENTIAL ); + else if (cache_hint == FILE_RANDOM_ACCESS) + posix_fadvise( fd->unix_fd, 0, 0, POSIX_FADV_RANDOM ); +#endif + if (root_fd != -1) fchdir( server_dir_fd ); /* go back to the server dir */ return fd; -- 2.25.1
August 3, 2021 10:31 AM, "Luke Deller" <luke(a)deller.id.au> wrote:
diff --git a/server/fd.c b/server/fd.c index b953da2ab85..b20fa97229c 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2036,6 +2037,15 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam free( closed_fd ); fd->cacheable = 1; } + +#if _POSIX_C_SOURCE > 200112L
This really needs a configure check. Under certain circumstances, _POSIX_C_SOURCE can be 200809L on macOS, but AFAIK it still doesn't have posix_fadvise(2/3) even on macOS 12.
+ cache_hint = options & (FILE_SEQUENTIAL_ONLY | FILE_RANDOM_ACCESS); + if (cache_hint == FILE_SEQUENTIAL_ONLY) + posix_fadvise( fd->unix_fd, 0, 0, POSIX_FADV_SEQUENTIAL ); + else if (cache_hint == FILE_RANDOM_ACCESS) + posix_fadvise( fd->unix_fd, 0, 0, POSIX_FADV_RANDOM ); +#endif + if (root_fd != -1) fchdir( server_dir_fd ); /* go back to the server dir */ return fd;
Chip
participants (3)
-
Chip Davis -
Luke Deller -
Marvin