f3read requires O_DIRECT on Cygwin to avoid reading cached data

[chrfranke]

The method used to avoid that f3read reads previously cached data does not work on Cygwin. This may result in false positive tests. Here is the local patch used for the new Cygwin package f3-8.0-2:

From c85e6f20d1de2c5cab358929e490d760e50e41d1 Mon Sep 17 00:00:00 2001
From: Christian Franke <christian.franke@t-online.de>
Date: Mon, 27 Jan 2025 12:04:17 +0100
Subject: [PATCH] f3read: ensure unbuffered reads also on Cygwin

Use O_DIRECT because POSIX_FADV_DONTNEED etc. have not the desired
effect.
---
 f3read.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/f3read.c b/f3read.c
index 7537b2e..fd670c5 100644
--- a/f3read.c
+++ b/f3read.c
@@ -1,5 +1,8 @@
 #define _POSIX_C_SOURCE 200112L
 #define _XOPEN_SOURCE 600
+#ifdef __CYGWIN__
+#define _BSD_SOURCE /* required for O_DIRECT */
+#endif
 
 #include <assert.h>
 #include <stdint.h>
@@ -243,16 +246,18 @@ static void validate_file(const char *path, int number, struct flow *fw,
 	printf("Validating file %s ... ", filename);
 	fflush(stdout);
 #ifdef __CYGWIN__
-	/* We don't need write access, but some kernels require that
-	 * the file descriptor passed to fdatasync(2) to be writable.
+	/* On Cygwin, the fdatasync() and posix_fadvise() calls below do
+	 * not have the desired effect.  Use O_DIRECT instead to avoid
+	 * read caching.
 	 */
-	fd = open(full_fn, O_RDWR);
+	fd = open(full_fn, O_RDONLY | O_DIRECT);
 #else
 	fd = open(full_fn, O_RDONLY);
 #endif
 	if (fd < 0)
 		err(errno, "Can't open file %s", full_fn);
 
+#ifndef __CYGWIN__
 	/* If the kernel follows our advice, f3read won't ever read from cache
 	 * even when testing small memory cards without a remount, and
 	 * we should have a better reading-speed measurement.
@@ -262,6 +267,7 @@ static void validate_file(const char *path, int number, struct flow *fw,
 
 	/* Help the kernel to help us. */
 	assert(!posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL));
+#endif
 
 	saved_errno = 0;
 	expected_offset = (uint64_t)number * GIGABYTES;
-- 
2.45.1

BTW: It may make sense to use the (non-POSIX) flag O_DIRECT on all platforms supporting it (Linux, *BSD, ...).

POSIX 2024 says about posix_fadvise(): The implementation may use this information to optimize handling of the specified data.

[AltraMayor]

Would changing the date in #define _POSIX_C_SOURCE 200112L from 200112L to 200809L avoid the need for #define _BSD_SOURCE?

Could you try implementing fake versions of fdatasync() and posix_fadvise() in utils.h as is done for other platforms?

I minimize peppering #ifdefs for platforms throughout the code to keep the code as readable as possible. If the two requests above are implemented, we don't have to add another #ifdef in f3read.c.

Could you issue a pull request to follow up on this issue?

[chrfranke]

Thanks for the feedback.

Would changing the date in #define _POSIX_C_SOURCE 200112L from 200112L to 200809L avoid the need for #define _BSD_SOURCE?

No. If this would work it would be a bug in the include files as O_DIRECT is not and never was present in any POSIX version or its predecessor SUS:
https://pubs.opengroup.org/onlinepubs/9799919799/functions/open.html
https://pubs.opengroup.org/onlinepubs/007908799/xsh/open.html

A quick research shows that the policies in the include files differ considerably:

• Newlib libc (used by Cygwin, RTEMS, ...): O_DIRECT is defined by default but disabled if specific API subsets are selected. The effective logic is equivalent to:

#if  !(defined __STRICT_ANSI__ || defined _POSIX_C_SOURCE || defined _XOPEN_SOURCE /* || even more */) \
   || (defined _DEFAULT_SOURCE || defined _BSD_SOURCE || defined _GNU_SOURCE)
#define O_DIRECT 0x80000
#endif

• GNU libc on Debian 12: O_DIRECT requires _GNU_SOURCE. _DEFAULT_SOURCE or _BSD_SOURCE are not sufficent. _BSD_SOURCE results in a deprecation warning which suggests _DEFAULT_SOURCE.

• Musl libc on Debian 12: O_DIRECT is defined unconditionally.

• FreeBSD 13: O_DIRECT is defined by default but disabled if (defined _ANSI_SOURCE || defined _C99_SOURCE || defined _C11_SOURCE).

• macOS/Darwin: O_DIRECT is unavailable (AFIAK).

Side note: With GCC, cc -std=c99 implicitly defines __STRICT_ANSI__=1 but -pedantic does not. With FreeBSD's default cc (aka clang), the opposite is the case.

Could you try implementing fake versions of fdatasync() and posix_fadvise() in utils.h as is done for other platforms?

Sorry, no. This is not possible because Cygwin offers no way to enable an O_DIRECT equivalent for an already opened fd.

I minimize peppering #ifdefs for platforms throughout the code to keep the code as readable as possible. If the two requests above are implemented, we don't have to add another #ifdef in f3read.c.

I fully agree that ugly #if...s should be avoided. Unfortunately, as the above observations suggest, this is not always possible, in particular for features not addressed by ISO C and/or POSIX.

Could you issue a pull request to follow up on this issue?

Possibly later when I have more spare time to investigate this. For now I only want to share the fix present in the Cygwin f3 package.

I'm still not convinced that the fdatasync() ... POSIX_FADV_DONTNEED ... POSIX_FADV_SEQUENTIAL method to prevent that f3read reads cached data from a previous f3write would work on most platforms. All POSIX_FADV_* are only hints to the OS and could be safely ignored. Using O_DIRECT if available may be an alternative.