Hi,
On 2025-01-26 12:55:15 -0800, Jacob Brazeal wrote:
> The patch series looks good.
Thanks for reviewing! And sorry for not getting back to you about your review
comments earlier.
> > StaticAssertDecl((MAX_BACKENDS & LW_FLAG_MASK) == 0,
> > "MAX_BACKENDS and LW_FLAG_MASK overlap");
>
> Should this check that MAX_BACKENDS & LW_LOCK_MASK == 0? To also ensure the
> LW_VAL_EXCLUSIVE bit does not overlap.
Yes, I think you're right.
On 2025-01-26 12:57:50 -0800, Jacob Brazeal wrote:
> Looking at v1-0003-WIP-Base-LWLock-limits-directly-on-MAX_BACKENDS.patch,
> I'm curious about the following assert;
>
> > #define LW_VAL_EXCLUSIVE (MAX_BACKENDS + 1)
> ...
> > StaticAssertDecl(MAX_BACKENDS < LW_VAL_EXCLUSIVE,
> "MAX_BACKENDS too big for lwlock.c");
>
> Since LW_VAL_EXCLUSIVE is already defined as MAX_BACKENDS + 1, is this
> basically just checking for an integer overflow?
You're right, it's redundant. I think I added that in an earlier version and
then didn't remove it.
Updated patches attached.
I didn't really like moving MAX_BACKENDS to pg_config_manual.h, there are too
many constraints on it. I moved it to procnumber.h. That's maybe not perfect,
but seems a bit better? I also introduced MAX_BACKENDS_BITS, so that it's
easier to check against when asserting restrictions on bit space.
I also added a patch from a different thread, to remove a bunch of the magic
constants in buf_internals.h, as it felt awkward to assert the refcount bits
alone were compatible with MAX_BACKENDS, when a 'standalone' 18 was used in a
bunch of other places. As Heikki had already reviewed that quite a while ago,
I'll push that soon.
I chose to not directly base BUF_REFCOUNT_BITS directly on MAX_BACKEND_BITS,
as it's ok to lower MAX_BACKEND_BITS without adjusting BUF_REFCOUNT_BITS. But
I went back and forth on that one.
Greetings,
Andres Freund
