On Sat, 28 Sept 2024 at 20:03, Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Tue, Feb 13, 2024 at 10:01 AM Dave Cramer <davecramer@postgres.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 Thomas, Thanks, I will be able to try this when I get back from NY. So Thurs or so. Dave
