Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers

Поиск
Список
Период
Сортировка
От Greg Burd
Тема Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers
Дата
Msg-id 8e926f26-0a97-434f-a665-8bb6f9159b08@app.fastmail.com
обсуждение исходный текст
Ответ на Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers  (Nathan Bossart <nathandbossart@gmail.com>)
Ответы Re: [PATCH] Fix ARM64/MSVC atomic memory ordering issues on Win11 by adding explicit DMB ?barriers
Список pgsql-hackers
On Fri, Dec 12, 2025, at 11:03 AM, Nathan Bossart wrote:
> +/*
> + * _InterlockedExchange() generates a full memory barrier (or release
> + * semantics that ensures all prior memory operations are visible to
> + * other cores before the lock is released.
> + */
> +#define S_UNLOCK(lock) (InterlockedExchange(lock, 0))

Nathan, thanks for looking at the patch!

> This seems to change the implementation from
>
>     #define S_UNLOCK(lock)    \
>         do { _ReadWriteBarrier(); (*(lock)) = 0; } while (0)
>
> in some cases, but I am insufficiently caffeinated to figure out what
> platforms use which implementation.  In any case, it looks like we are
> changing it for some currently-supported platforms, and I'm curious why.

This change is within _MSC_VER, but AFAICT this intrinsic is available across their supported platforms.  The previous
implementationof S_UNLOCK() boils down to a no-op because the _ReadWriteBarrier()[1] is a hint to the compiler and does
notemit any instruction on any platform and it's also deprecated.  So, on MSVC S_UNLOCK is an unguarded assignment and
thena loop that will be optimized out, not really what we wanted I'd imagine.  My tests with godbolt showed this to be
true,no instruction barriers emitted.  I think it was Andres[2] who suggested replacing it with
_InterlockedExchange()[3]. So, given that _ReadWriteBarrier() is deprecated I decided not to specialize this change to
onlythe ARM64 platform, sorry for not making that clear in the commit or email.
 

My buildfarm animal for this platform was just approved and is named "unicorn", I'm not making that up. :)

best.

-greg

> Perhaps there's some way to make the #ifdefs a bit more readable, too
> (e.g., a prerequisite patch that rearranges things).
>
> The rest looks generally reasonable to me.
>
> -- 
> nathan

[1] https://learn.microsoft.com/en-us/cpp/intrinsics/readwritebarrier
[2] https://www.postgresql.org/message-id/beirrgqo5n5e73dwa4dsdnlbtef3bsdv5sgarm6przdzxvifk5%40whyuhyemmhyr
[3] https://learn.microsoft.com/en-us/cpp/intrinsics/interlockedexchange-intrinsic-functions



В списке pgsql-hackers по дате отправления: