On 04/08/2026 17:32, Andres Freund wrote:
> On 2026-08-04 16:07:58 +0200, Peter Eisentraut wrote:
>> On 23.07.26 21:56, Nathan Bossart wrote:
>>> On Wed, Jul 22, 2026 at 09:31:49AM -0400, Nathan Bossart wrote:
>>>> Ah. It looks like we could use 0 as the sentinel and simplify the call
>>>> sites. They subtract one before calling SetStartupBufferPinWaitBufId() and
>>>> add one after calling GetStartupBufferPinWaitBufId().
>>>
>>> I added a new prerequisite patch (v2-0004) that does this.
>>
>> Maybe this is okay, but there are a bunch more places (not touched by your
>> patches) that mix unsigned atomics operations with actually signed values.
>> Stuff like PIDs and proc numbers. I think for better overall hygiene and to
>> simplify broader adoption, perhaps we should introduce support for signed
>> atomic variables.
>
> I'm quite hesitant to do that, at least without a lot more clear cut examples
> where it actually would make the code better. I think it's rarely a good idea
> to use signed variables for atomics, because you get undefined behaviour on
> overflow, there's problems with bit masking, etc. IME most data in atomically
> modified should actually be unsigned and probably should have been unsigned
> before the conversion to atomics.
We could provide pg_atomic_read/write_i32() and
pg_atomic_compare_exchange_i32() but leave out fetch-and-add and other
such instructions that have overflow or bit masking issues.
- Heikki