[PATCH v2 1/4] msvcrt/tests: Add test for fopen sequential access hint
Test that adding "S" to the fopen mode results in the underlying file handle having the sequential access hint set. Signed-off-by: Luke Deller <luke(a)deller.id.au> --- dlls/msvcrt/tests/file.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/dlls/msvcrt/tests/file.c b/dlls/msvcrt/tests/file.c index fb242d81282..d02087be46c 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,39 @@ static void test_lseek(void) DeleteFileA("_creat.tst"); } +static void test_fopen_hints(void) +{ + char *tempf; + FILE *fp; + HANDLE handle; + FILE_MODE_INFORMATION mode_info; + IO_STATUS_BLOCK io; + NTSTATUS status; + + /* 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); + + /* open test file with sequential access hint */ + fp = fopen(tempf, "rbS"); + ok(fp != NULL, "unable to open test file with sequential access hint\n"); + + /* check that sequential access hint is set on underlying file handle */ + handle = (HANDLE)_get_osfhandle(_fileno(fp)); + status = NtQueryInformationFile(handle, &io, &mode_info, sizeof(mode_info), + FileModeInformation); + ok(!status, "NtQueryInformationFile failed\n"); + ok(mode_info.Mode & FILE_SEQUENTIAL_ONLY, "handle missing hint\n"); + + /* clean up */ + fclose(fp); + unlink(tempf); + free(tempf); +} + START_TEST(file) { int arg_c; @@ -2803,6 +2837,7 @@ START_TEST(file) test_close(); test__creat(); test_lseek(); + test_fopen_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> --- dlls/msvcrt/file.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/dlls/msvcrt/file.c b/dlls/msvcrt/file.c index 6e955099090..716d397d246 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,10 @@ static int msvcrt_get_flags(const wchar_t* mode, int *open_flags, int* stream_fl case 'w': break; case 'S': + *open_flags |= _O_SEQUENTIAL; + break; case 'R': - FIXME("ignoring cache optimization flag: %c\n", mode[-1]); + *open_flags |= _O_RANDOM; break; default: ERR("incorrect mode flag: %c\n", mode[-1]); @@ -2269,6 +2269,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
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=94840 Your paranoid android. === debiant2 (32 bit report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Chinese:China report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit WoW report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (64 bit WoW report) === msvcrt: file.c:2760: Test failed: handle missing hint
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 +++ 1 file changed, 3 insertions(+) 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); -- 2.25.1
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> --- v2: spotted a typo: set the offset argument of posix_fadvise to 0 not 1 (this has no effect on linux, where the call affects the entire file and not just the region specified) --- server/fd.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/fd.c b/server/fd.c index b953da2ab85..873a94b048a 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2036,6 +2036,14 @@ 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 + if (options & FILE_SEQUENTIAL_ONLY) + posix_fadvise( fd->unix_fd, 0, 0, POSIX_FADV_SEQUENTIAL ); + else if (options & 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
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=94839 Your paranoid android. === debiant2 (32 bit report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Arabic:Morocco report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit German report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit French report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Hebrew:Israel report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Hindi:India report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Japanese:Japan report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit Chinese:China report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (32 bit WoW report) === msvcrt: file.c:2760: Test failed: handle missing hint === debiant2 (64 bit WoW report) === msvcrt: file.c:2760: Test failed: handle missing hint
Hi Luke, On 8/2/21 5:03 PM, Luke Deller wrote:
+static void test_fopen_hints(void) +{ + char *tempf; + FILE *fp; + HANDLE handle; + FILE_MODE_INFORMATION mode_info; + IO_STATUS_BLOCK io; + NTSTATUS status; + + /* 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); + + /* open test file with sequential access hint */ + fp = fopen(tempf, "rbS"); + ok(fp != NULL, "unable to open test file with sequential access hint\n"); + + /* check that sequential access hint is set on underlying file handle */ + handle = (HANDLE)_get_osfhandle(_fileno(fp)); + status = NtQueryInformationFile(handle, &io, &mode_info, sizeof(mode_info), + FileModeInformation); + ok(!status, "NtQueryInformationFile failed\n"); + ok(mode_info.Mode & FILE_SEQUENTIAL_ONLY, "handle missing hint\n"); + + /* clean up */ + fclose(fp); + unlink(tempf); + free(tempf); +} It would be nice to also test fp = fopen(tempf, "rbRS"); and fp = fopen(tempf, "rbSR"); case.
The same applies to second patch, what happens if _O_RANDOM and _O_SEQUENTIAL is passed? You will also need to mark the tests failing in wine with todo_wine or reorder the patches. Thanks, Piotr
participants (3)
-
Luke Deller -
Marvin -
Piotr Caban