From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Dave Cramer <davecramer(at)postgres(dot)rocks> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, Anthony Roberts <anthony(dot)roberts(at)linaro(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Lina Iyer <lina(dot)iyer(at)linaro(dot)org>, Mike Holmes <mike(dot)holmes(at)linaro(dot)org>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: [PATCH] Add native windows on arm64 support |
Date: | 2024-09-29 00:02:21 |
Message-ID: | CA+hUKG+2E-yQ-zg5pvUAh8QPfOF6R=yf_PiKXJ0o+j7yV_oSuQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Feb 13, 2024 at 10:01 AM Dave Cramer <davecramer(at)postgres(dot)rocks> wrote:
> > postgres.exe!dsa_free(dsa_area * area, unsigned __int64 dp) Line 869 C
> postgres.exe!resize(dshash_table * hash_table, unsigned __int64 new_size_log2) Line 879 C
> postgres.exe!dshash_find_or_insert(dshash_table * hash_table, const void * key, bool * found) Line 480 C
> postgres.exe!pgstat_get_entry_ref(PgStat_Kind kind, unsigned int dboid, unsigned int objoid, bool create, bool * created_entry) Line 455 C
Hmm, we can see that the spinlock stuff is too weak (assumes TSO), but
there aren't any spinlocks near that code so I think something else is
wrong. What does pg_read_barrier() compile to? I mention that
because it's used near there in a somewhat complicated way. But I'm a
bit confused because by reading through the code it looks like it
should be too strong, not too weak. I think it should fall back to
pg_memory_barrier_impl() for lack of pg_read_barrier_impl(), which
should produce MemoryBarrier() and I guess DMB SY (ARM instruction for
full system data memory barrier). So maybe some other pg_atomic_XXX
operations are falling back to code that doesn't work.
Just for experimentation, you could see what happens if you redirect
all that stuff to C11's <stdatomic.h>. Here's a quick-and-dirty patch
for that, which works on my ARM Mac, but I have no idea if it'll work
on MSVC (our CI uses MSVC 2019; they only added <stdatomic.h> in MSVC
2022, but that's the same edition that added ARM so you must have it).
It's not a very finely tuned patch (for example I think several calls
should use the _explicit() variants and be weaker, but they err on the
side of being too strong, so the code they generate is probably not as
fast as it could be; I lost interested when the general idea was
rejected for now, but it might help learn something about the MSVC+ARM
situation).
Combined with the patch that redirects spinlocks to atomics[1], there
would be no 'hand rolled' code relating to memory order or atomics,
just standard modern C. There are obviously other architecture tests
here and there, and some of them will be wrong (some of them are wrong
already for MSVC on x86), and that might be fixed by project
standardisation[2].
[1] https://www.postgresql.org/message-id/CA%2BhUKGJ%2BoA%2B62iUZ-EQb5R2cAOW3Y942ZoOtzOD%3D1sQ05iNg6Q%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CA%2BhUKG%2BOu%2BC%2BpXioo_W1gKcwRCEN1WNrLBBPSMJyxjgc%2B1JEJA%40mail.gmail.com
Attachment | Content-Type | Size |
---|---|---|
0001-Optionally-do-port-atomics.h-with-stdatomic.h.patch | application/octet-stream | 8.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2024-09-29 01:19:11 | Re: First draft of PG 17 release notes |
Previous Message | Tom Lane | 2024-09-28 22:59:02 | Re: pg_verifybackup: TAR format backup verification |