Re: [PATCH] bufmgr: tighten LWLock:BufferMapping on InvalidateBuffer

[Alexandre Felipe]

Hi Hackers,

Scrolling lwlock.c I noticed this piece of code in LWLockAttemptLock

 if (mode == LW_EXCLUSIVE)
 {
   lock_free = (old_state & LW_LOCK_MASK) == 0;
   if (lock_free)
     desired_state += LW_VAL_EXCLUSIVE;
 }
 else
 {
   lock_free = (old_state & LW_VAL_EXCLUSIVE) == 0;
   if (lock_free)
    desired_state += LW_VAL_SHARED;
 }

Where LW_LOCK_MASK is [xsss...ss], i.e. one bit for exclusive lock
followed by multiple bits for shared locks.

When acquiring shared locks LW_VAL_EXCLUSIVE times, we end up
with an exclusive lock because the shared lock count overflows to the
shared lock [0111...11] + [0000...01] = [1000..00].

That is probably fine, it will stop us from getting exclusive locks or any
additional shared locks.

The mode is stored as LW_SHARED
held_lwlocks[num_held_lwlocks++].mode = mode;

So the release will revert the counter correctly
   mode = held_lwlocks[i].mode;
   ...
   if (mode == LW_EXCLUSIVE)
    oldstate = pg_atomic_sub_fetch_u32(&lock->state, LW_VAL_EXCLUSIVE);
  else
    oldstate = pg_atomic_sub_fetch_u32(&lock->state, LW_VAL_SHARED);

But the LWLock state of LW_VAL_EXCLUSIVE shared locks is identical to
an LWLock with one exclusive lock.

I didn't see (so far) mentions of this (clever?) behaviour.
Is it undocumented feature, or something that should be fixed?

That would be a one line fix under the current assumptions
(old_state & LW_LOCK_MASK) + LW_VAL_SHARED < LW_VAL_EXCLUSIVE;

Regards,
Alexandre