On 25/02/2024 00:37, Thomas Munro wrote:
On Sun, Feb 25, 2024 at 11:16 AM Thomas Munro <thomas.mu...@gmail.com> wrote:
On Sun, Feb 25, 2024 at 11:06 AM Heikki Linnakangas <hlinn...@iki.fi> wrote:
Regarding the issue at hand, perhaps we should define PG_IO_ALIGN_SIZE as 16 on
AIX, if that's the best the linker can do on that platform.
You'll probably get either an error or silently fall back to buffered
I/O, if direct I/O is enabled and you try to read/write a badly
aligned buffer. That's documented (they offer finfo() to query it,
but it's always 4KB for the same sort of reasons as it is on every
other OS).
I guess it's the latter ("to work efficiently" sounds like it isn't
going to reject the request):
https://www.ibm.com/docs/en/aix/7.3?topic=tuning-direct-io
If you make it < 4KB then all direct I/O would be affected, not just
this one place, so then you might as well just not allow direct I/O on
AIX at all, to avoid giving a false impression that it does something.
(Note that if we think the platform lacks O_DIRECT we don't make those
assertions about alignment).
FWIW I'm aware of one other thing that is wrong with our direct I/O
support on AIX: it should perhaps be using a different flag. I
created a wiki page to defer thinking about any AIX issues
until/unless at least one real, live user shows up, which hasn't
happened yet: https://wiki.postgresql.org/wiki/AIX
Here's a patch that effectively disables direct I/O on AIX. I'm inclined
to commit this as a quick fix to make the buildfarm green again.
I agree with Andres though, that unless someone raises their hand and
volunteers to properly maintain the AIX support, we should drop it. The
current AIX buildfarm members are running AIX 7.1, which has been out of
support since May 2023
(https://www.ibm.com/support/pages/aix-support-lifecycle-information).
See also older thread on this [0].
Noah, you're running the current AIX buildfarm animals. How much effort
are you interested to put into AIX support?
[0]
https://www.postgresql.org/message-id/20220702183354.a6uhja35wta7agew%40alap3.anarazel.de
--
Heikki Linnakangas
Neon (https://neon.tech)
From 3e206b952b8011c4bab97c7c87ea693832137999 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Sun, 25 Feb 2024 13:33:07 +0200
Subject: [PATCH 1/1] Disable O_DIRECT on AIX
AIX apparently doesn't support the __attribute__((aligned(a)))
attribute, when a > 16. That means that PGIOAlignedBlock is not
correctly aligned. That showed up as an assertion failure when writing
pages with the new bulk_write.c facility, because it uses "static
const" of type PGIOAlignedBlock.
pg_prewarm.c also uses a static variable of type PGIOAlignedBlock, and
it's not clear why it hasn't tripped the assertion.
There surely would be ways to force the alignment on AIX, but at least
this should make the buildfarm green again, and it doesn't seem worth
spending more effort on this right now. We might soon drop AIX support
altogether unless someone steps up to the plate to maintain it
properly and set up a more recent buildfarm animal for it.
Discussion: https://www.postgresql.org/message-id/20240224195024...@rfd.leadboat.com
---
src/include/c.h | 14 ++++++++++----
src/include/port/aix.h | 3 +++
src/include/storage/fd.h | 2 +-
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/include/c.h b/src/include/c.h
index 2e3ea206e1..9a464bd592 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -186,6 +186,12 @@
#define pg_attribute_noreturn() __attribute__((noreturn))
#define pg_attribute_packed() __attribute__((packed))
#define HAVE_PG_ATTRIBUTE_NORETURN 1
+
+/* Some platforms (AIX) support the aligned attribute only for small values */
+#if !defined(PG_MAX_ATTRIBUTE_ALIGN) || PG_MAX_ATTRIBUTE_ALIGN >= PG_IO_ALIGN_SIZE
+#define pg_attribute_io_aligned __attribute__((aligned(PG_IO_ALIGN_SIZE)))
+#endif
+
#elif defined(_MSC_VER)
/*
* MSVC supports aligned. noreturn is also possible but in MSVC it is
@@ -1123,8 +1129,8 @@ typedef union PGAlignedBlock
*/
typedef union PGIOAlignedBlock
{
-#ifdef pg_attribute_aligned
- pg_attribute_aligned(PG_IO_ALIGN_SIZE)
+#ifdef pg_attribute_io_aligned
+ pg_attribute_io_aligned
#endif
char data[BLCKSZ];
double force_align_d;
@@ -1134,8 +1140,8 @@ typedef union PGIOAlignedBlock
/* Same, but for an XLOG_BLCKSZ-sized buffer */
typedef union PGAlignedXLogBlock
{
-#ifdef pg_attribute_aligned
- pg_attribute_aligned(PG_IO_ALIGN_SIZE)
+#ifdef pg_attribute_io_aligned
+ pg_attribute_io_aligned
#endif
char data[XLOG_BLCKSZ];
double force_align_d;
diff --git a/src/include/port/aix.h b/src/include/port/aix.h
index 5b1159c578..405151746d 100644
--- a/src/include/port/aix.h
+++ b/src/include/port/aix.h
@@ -12,3 +12,6 @@
#if defined(__ILP32__) && defined(__IBMC__)
#define PG_FORCE_DISABLE_INLINE
#endif
+
+/* pg_attribute_aligned(n) works only up to 16 bytes on AIX. */
+#define PG_MAX_ATTRIBUTE_ALIGN 16
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 60bba5c970..579cb3142b 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -88,7 +88,7 @@ extern PGDLLIMPORT int max_safe_fds;
* idea on a Unix). We can only use it if the compiler will correctly align
* PGIOAlignedBlock for us, though.
*/
-#if defined(O_DIRECT) && defined(pg_attribute_aligned)
+#if defined(O_DIRECT) && defined(pg_attribute_io_aligned)
#define PG_O_DIRECT O_DIRECT
#elif defined(F_NOCACHE)
#define PG_O_DIRECT 0x80000000
--
2.39.2
