| From: | "Greg Burd" <greg(at)burd(dot)me> |
|---|---|
| To: | "Thomas Munro" <thomas(dot)munro(at)gmail(dot)com> |
| Cc: | "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Nathan Bossart" <nathandbossart(at)gmail(dot)com>, "Peter Eisentraut" <peter(at)eisentraut(dot)org>, "Dave Cramer" <davecramer(at)gmail(dot)com>, "Andres Freund" <andres(at)anarazel(dot)de> |
| Subject: | Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB barriers |
| Date: | 2025-12-11 16:16:28 |
| Message-ID: | 1c2a29b8-5b1e-44f7-a871-71ec5fefc120@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Dec 10, 2025, at 4:31 PM, Thomas Munro wrote:
> On Thu, Dec 11, 2025 at 5:32 AM Greg Burd <greg(at)burd(dot)me> wrote:
>> Rebased with only minor changes to meson.build this patch is ready for review/commit as it is passing tests on my aarch64 Win11 MSVC system. Also note that this system I'm testing on is ready to become a member of the buildfarm (application submitted) and monitor this combo in perpetuity.
>
> - if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and
> __crc32cd without -march=armv8-a+crc',
> ...
> + if host_machine.cpu_family() == 'aarch64'
Thanks for taking the time to review.
> I think this new nesting of the CRC32 feature tests breaks the test on
> "armv7" distros (in our build farm, that's a bunch of RPis running
> Debian/Raspbian, but at least FreeBSD and NetBSD also support
> "armv7"). Any ARM chip made since around 2011 is really an ARMv8+
> chip running Aarch32 code and can thus reach the ARMv8 instructions.
> For example "grison" says:
>
> checking build system type... (cached) armv7l-unknown-linux-gnueabihf
> ...
> checking which CRC-32C implementation to use... ARMv8 CRC instructions
> with runtime check
That was rather silly of me to overlook, apologies. I went back to the code that was there before written by Andres I believe and simply added a test for this one special case. That reduces code churn and simplifies the logic while preserving the earlier behavior.
As I was re-reading I decided to review the YEILD vs ISB question for this platform combo again in light of a82a5ee [1]. What I found was a wealth of information and work on the topic and the thread mentioned MariaDB so I did some digging (because, why not?). Go-lang[2] uses ISB but a comment there led me on to another resource, a blog post from ARM on the topic of spin locks for ARM64[3]. I found that like Go-lang, Rust[4] uses ISB (and if you read only one thread on the topic this is the one to dig into). The developers on the thread wrote a number of benchmarks and tested a variety of approaches landing on "ISB SY" on aarch64. I found that MySQL[5][6] also went with ISB, as has Cloudera[7], Impala[8], Aerospike[9], and WebAssembly[10] at which point I stopped looking around. There were other resources out there for understanding ARM and relaxed memory models such as [11], [12], and [13] for those that care to learn.
All of this is to say that I swapped out the YEILD for "ISB SY" for the ARM64/MSVC because that seems to make the most sense from the evidence I found and for consistency in our code.
> -#define S_UNLOCK(lock) \
> +#define S_UNLOCK(lock) \
>
> Bogus whitespace change.
Fixed, thanks.
best.
-greg
[1] https://postgr.es/m/78338F29-9D7F-4DC8-BD71-E9674CE71425@amazon.com
[2] https://github.com/golang/go/issues/69232
[3] https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/multi-threaded-applications-arm
[4] https://github.com/rust-lang/rust/commit/c064b6560b7ce0adeb9bbf5d7dcf12b1acb0c807
[5] https://bugs.mysql.com/bug.php?id=100664
[6] https://github.com/mysql/mysql-server/pull/305/files
[7] https://gerrit.cloudera.org/#/c/19865/
[8] https://issues.apache.org/jira/browse/IMPALA-12122
[9] https://github.com/aerospike/aerospike-common/pull/17
[10] https://github.com/WebAssembly/threads/issues/15
[11] https://developer.arm.com/documentation/genc007826/latest "Barrier Litmus Tests and Cookbook"
[12] http://www.rdrop.com/users/paulmck/scalability/paper/whymb.2010.07.23a.pdf - Memory Barriers: a Hardware View for Software Hackers
[13] https://randomascii.wordpress.com/2020/11/29/arm-and-lock-free-programming/
| Attachment | Content-Type | Size |
|---|---|---|
| v7-0001-Enable-the-Microsoft-Windows-ARM64-MSVC-platform.patch | application/octet-stream | 7.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Heikki Linnakangas | 2025-12-11 16:30:35 | Re: Visibility bug in tuple lock |
| Previous Message | Mahendra Singh Thalor | 2025-12-11 16:09:39 | Re: Non-text mode for pg_dumpall |