On Tue, Jul 20, 2021 at 2:13 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > Hmm ... we used to have to avoid putting #if constructs in the arguments > of macros (such as StaticAssertStmt). Maybe that's not a thing anymore > with C99, and in any case this whole stanza is fairly platform-specific > so we may not run into a compiler that complains. But my hindbrain wants > to see this done with separate statements, eg > > #if defined(O_CLOEXEC) > StaticAssertStmt((PG_O_DIRECT & O_CLOEXEC) == 0, > "PG_O_DIRECT collides with O_CLOEXEC"); > #endif
Ok, done. While I was here again, I couldn't resist trying to extend this to Solaris, since it looked so easy. I don't have access, but I tested on Illumos by undefining O_DIRECT. Thoughts?
From 3016ede1dfd972badac65954d6e908f77e3c134b Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Mon, 19 Jul 2021 21:47:16 +0000 Subject: [PATCH 1/2] Support direct I/O on Solaris. Extend the change done for macOS by commit 2dbe8905 to cover Solaris. Note that this doesn't affect Illumos (it gained O_DIRECT). Discussion: https://postgr.es/m/CA%2BhUKG%2BADiyyHe0cun2wfT%2BSVnFVqNYPxoO6J9zcZkVO7%2BNGig%40mail.gmail.com --- src/backend/storage/file/fd.c | 19 +++++++++++++++---- src/bin/pg_test_fsync/pg_test_fsync.c | 27 +++++++++++++++++++++------ src/include/storage/fd.h | 9 ++++++--- 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 5d5e8ae94e..78ac2caa8f 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -1057,11 +1057,13 @@ BasicOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode) int fd; tryAgain: -#ifdef PG_O_DIRECT_USE_F_NOCACHE +#if defined(PG_O_DIRECT_USE_F_NOCACHE) || \ + defined(PG_O_DIRECT_USE_DIRECTIO_ON) /* * The value we defined to stand in for O_DIRECT when simulating it with - * F_NOCACHE had better not collide with any of the standard flags. + * an extra system call had better not collide with any of the standard + * flags. */ StaticAssertStmt((PG_O_DIRECT & (O_APPEND | @@ -1089,10 +1091,19 @@ tryAgain: if (fd >= 0) { -#ifdef PG_O_DIRECT_USE_F_NOCACHE +#if defined(PG_O_DIRECT_USE_F_NOCACHE) || \ + defined(PG_O_DIRECT_USE_DIRECTIO_ON) if (fileFlags & PG_O_DIRECT) { - if (fcntl(fd, F_NOCACHE, 1) < 0) + int rc; + +#if defined(PG_O_DIRECT_USE_F_NOCACHE) + rc = fcntl(fd, F_NOCACHE, 1); +#endif +#if defined(PG_O_DIRECT_USE_DIRECTIO_ON) + rc = directio(fd, DIRECTIO_ON); +#endif + if (rc < 0) { int save_errno = errno; diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c index fef31844fa..f5e3868c10 100644 --- a/src/bin/pg_test_fsync/pg_test_fsync.c +++ b/src/bin/pg_test_fsync/pg_test_fsync.c @@ -221,6 +221,8 @@ handle_args(int argc, char *argv[]) printf(_("O_DIRECT supported on this platform for open_datasync and open_sync.\n")); #elif defined(F_NOCACHE) printf(_("F_NOCACHE supported on this platform for open_datasync and open_sync.\n")); +#elif defined(DIRECTIO_ON) + printf(_("DIRECTIO_ON supported on this platform for open_datasync and open_sync.\n")); #else printf(_("Direct I/O is not supported on this platform.\n")); #endif @@ -271,14 +273,27 @@ open_direct(const char *path, int flags, mode_t mode) fd = open(path, flags, mode); -#if !defined(O_DIRECT) && defined(F_NOCACHE) - if (fd >= 0 && fcntl(fd, F_NOCACHE, 1) < 0) +#if !defined(O_DIRECT) && \ + (defined(F_NOCACHE) || \ + defined(DIRECTIO_ON)) + if (fd >= 0) { - int save_errno = errno; + int rc; - close(fd); - errno = save_errno; - return -1; +#if defined(F_NOCACHE) + rc = fcntl(fd, F_NOCACHE, 1); +#endif +#if defined(DIRECTIO_ON) + rc = directio(fd, DIRECTIO_ON); +#endif + if (rc < 0) + { + int save_errno = errno; + + close(fd); + errno = save_errno; + return -1; + } } #endif diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index 2d843eb992..b04988f818 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -82,15 +82,18 @@ extern int max_safe_fds; /* * O_DIRECT is not standard, but almost every Unix has it. We translate it * to the appropriate Windows flag in src/port/open.c. We simulate it with - * fcntl(F_NOCACHE) on macOS inside fd.c's open() wrapper. We use the name - * PG_O_DIRECT rather than defining O_DIRECT in that case (probably not a good - * idea on a Unix). + * extra calls on macOS and Solaris inside fd.c's open() wrapper. We use the + * name PG_O_DIRECT rather than defining O_DIRECT in that case (probably not a + * good idea on a Unix). */ #if defined(O_DIRECT) #define PG_O_DIRECT O_DIRECT #elif defined(F_NOCACHE) #define PG_O_DIRECT 0x80000000 #define PG_O_DIRECT_USE_F_NOCACHE +#elif defined(DIRECTIO_ON) +#define PG_O_DIRECT 0x80000000 +#define PG_O_DIRECT_USE_DIRECTIO_ON #else #define PG_O_DIRECT 0 #endif -- 2.30.2
From 7c99892354500f046995acddc0789879a50a1096 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Mon, 19 Jul 2021 22:26:34 +0000 Subject: [PATCH 2/2] XXX Test --- src/backend/storage/file/fd.c | 2 ++ src/bin/pg_test_fsync/pg_test_fsync.c | 2 ++ src/include/storage/fd.h | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c index 78ac2caa8f..248e2c3cd4 100644 --- a/src/backend/storage/file/fd.c +++ b/src/backend/storage/file/fd.c @@ -1098,9 +1098,11 @@ tryAgain: int rc; #if defined(PG_O_DIRECT_USE_F_NOCACHE) + elog(LOG, "XXX F_NOCACHE"); /* HACK */ rc = fcntl(fd, F_NOCACHE, 1); #endif #if defined(PG_O_DIRECT_USE_DIRECTIO_ON) + elog(LOG, "XXX DIRECTIO_ON"); /* HACK */ rc = directio(fd, DIRECTIO_ON); #endif if (rc < 0) diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c index f5e3868c10..7396886cec 100644 --- a/src/bin/pg_test_fsync/pg_test_fsync.c +++ b/src/bin/pg_test_fsync/pg_test_fsync.c @@ -17,6 +17,8 @@ #include "common/logging.h" #include "getopt_long.h" +#undef O_DIRECT /* HACK: make Illumos behave like Solaris */ + /* * put the temp files in the local directory * unless the user specifies otherwise diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h index b04988f818..2c09639cf1 100644 --- a/src/include/storage/fd.h +++ b/src/include/storage/fd.h @@ -79,6 +79,8 @@ extern int max_safe_fds; #define FILE_POSSIBLY_DELETED(err) ((err) == ENOENT || (err) == EACCES) #endif +#undef O_DIRECT /* HACK: make Illumos behave like Solaris */ + /* * O_DIRECT is not standard, but almost every Unix has it. We translate it * to the appropriate Windows flag in src/port/open.c. We simulate it with -- 2.30.2
