> On Jan 15, 2026, at 07:37, Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2026-01-15 07:20:27 +0800, Chao Li wrote:
>>> On Jan 15, 2026, at 00:30, Andres Freund <[email protected]> wrote:
>>> On 2026-01-14 11:41:19 +0800, Chao Li wrote:
>>>> Basically, code changes in 0003 is straightforward, just a couple of small
>>>> comments:
>>>>
>>>> 1
>>>> ```
>>>> - * refcounts in buf_internals.h. This limitation could be lifted by
>>>> using a
>>>> - * 64bit state; but it's unlikely to be worthwhile as 2^18-1 backends
>>>> exceed
>>>> - * currently realistic configurations. Even if that limitation were
>>>> removed,
>>>> - * we still could not a) exceed 2^23-1 because inval.c stores the
>>>> ProcNumber
>>>> - * as a 3-byte signed integer, b) INT_MAX/4 because some places compute
>>>> - * 4*MaxBackends without any overflow check. We check that the configured
>>>> - * number of backends does not exceed MAX_BACKENDS in
>>>> InitializeMaxBackends().
>>>> + * refcounts in buf_internals.h. This limitation could be lifted, but
>>>> it's
>>>> ```
>>>>
>>>> Before this patch, there was room for lifting the limitation. With this
>>>> patch, state is 64bit already, but the significant 32bit will be used for
>>>> buffer locking as stated in buf_internals.h, in other words, there is no
>>>> room for lifting the limitation now. If that’s true, then I think we can
>>>> remove the statements about lifting limitation.
>>>
>>> I'm not following - there's plenty space for more bits if we need that:
>>>
>>> * State of the buffer itself (in order):
>>> * - 18 bits refcount
>>> * - 4 bits usage count
>>> * - 12 bits of flags
>>> * - 18 bits share-lock count
>>> * - 1 bit share-exclusive locked
>>> * - 1 bit exclusive locked
>>>
>>> That's 54 bits in total. Which part is in the lower and which in the upper
>>> 32bit isn't relevant for anything afaict?
>>
>> Because I saw the comment in buf_internals.h:
>> ```
>> * NB: A future commit will use a significant portion of the remaining bits to
>> * implement buffer locking as part of the state variable.
>> ```
>> That seems to indicate all the significant 32 bits will be used for buffer
>> locking.
>
> A significant portion != all. As the above excerpt from the comment shows, the
> locking uses 20 bits. We could increase max backends by 5 bits without running
> out of bits (we'd need space both in the refcount bitspace as well as the
> share-lock bitspace).
Make sense. I think I misread the comment.
>
>
>> Also, there is an assert that concretes the impression:
>> ```
>> StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS ==
>> 32,
>> "parts of buffer state space need to equal 32");
>> ```
>
> You can see that being relaxed in the subsequent commit, when we start to use
> more bits.
>
Sure. I plan to review 0003-0005 today. I believe I will get better
understanding.
So, 0001 and 0002 LGTM now.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
