While we are on the topic of comments from lwlock.c, there is one other one that confused me, in LWLockWaitListLock:
/* and then spin without atomic operations until lock is released */
{
SpinDelayStatus delayStatus;
init_local_spin_delay(&delayStatus);
while (old_state & LW_FLAG_LOCKED)
{
perform_spin_delay(&delayStatus);
old_state = pg_atomic_read_u32(&lock->state);
}
#ifdef LWLOCK_STATS
delays += delayStatus.delays;
#endif
finish_spin_delay(&delayStatus);
}
It seems that we are using an atomic operation in the loop (though, no compare-and-set, etc.) I might be mis-reading the intent of the comment, but I'm curious if there's a way to reword it, too.
Thinking a bit further about this, the purpose of the LW_SHARED_MASK section of the state is to count the number of lock-sharers. Thus, we only care about the actual number of backends (up to 2^18-1) here and not the size of the ProcNumber data type. So I do think the comment should read 2^18-1 and not 2^23-1. Here is a patch to that effect.
Hello all,
In lwlocks.c, we have the following comment, related to LWLock state:
/* Must be greater than MAX_BACKENDS - which is 2^23-1, so we're fine. */
#define LW_SHARED_MASK ((uint32) ((1 << 24)-1))
However, MAX_BACKENDS is set to 2^18-1. Here is the comment in postmaster.h:
/*
* Note: MAX_BACKENDS is limited to 2^18-1 because that's the width reserved
* for buffer references 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. This is rechecked in the
* relevant GUC check hooks and in RegisterBackgroundWorker().
*/
#define MAX_BACKENDS 0x3FFFF
2^23-1 is noted as an additional upper limit, but I wonder if it'd be correct to update the comment in lwlocks.c to
/* Must be greater than MAX_BACKENDS - which is 2^18-1, so we're fine. */
I'm not sure if this could lead to us actually saving some bits in the lwlock state, or if we could do anything useful with them anyway.
Jacob
