[PATCH v2 1/4] msvcrt/tests: Add test for fopen sequential access hint

[Luke Deller]

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 
---
 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

[PATCH v2 2/4] msvcrt: Translate file open access pattern hints

[Luke Deller]

Translate access pattern hints supplied to fopen / _open into the
corresponding attributes for the lower level CreateFileW call.

Signed-off-by: Luke Deller 
---
 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);

[PATCH v2 2/4] msvcrt: Translate file open access pattern hints

[Marvin]

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

[PATCH v2 3/4] kernelbase: Translate FILE_FLAG_SEQUENTIAL_SCAN

[Luke Deller]

Translate FILE_FLAG_SEQUENTIAL_SCAN into the corresponding flag for
NtCreateFile options

Signed-off-by: Luke Deller 
---
 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);

[PATCH v2 4/4] server: Implement file access hints using posix_fadvise

[Luke Deller]

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 
---
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;

[Marvin]

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

[Piotr Caban]

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