> I believe it refers to the state of the lock prior to lock acquisition;
not prior to subtraction.
That definitely makes sense as a way to read this variable in context, but
after reviewing other usages of old_state in lwlock.c, I tend to think that
this is an outlier usage and maybe the naming was just copy-pasted from one
of the other compare-and-swaps, which tend to use "old state" to mean "the
state just before we mutated it.". Here are three other usages that make me
think this:
First, in LWLockWaitListLock, we have:
old_state = pg_atomic_fetch_or_u32(&lock->state, LW_FLAG_LOCKED);
Here old_state contains the lock state *before* the compare and swap, the
opposite of the case we considered above, but it does also work out to "the
state prior to lock acquisition".
Next, in LWLockWaitListUnlock, we have:
old_state = pg_atomic_fetch_and_u32(&lock->state, ~LW_FLAG_LOCKED);
Assert(old_state & LW_FLAG_LOCKED);
... so in this case "old_state" means "the locked state just prior to the
unlock."
Third, In LWLockWaitListUnlock, we have:
old_state = pg_atomic_read_u32(&lock->state);
which also refers to the locked state, prior to maybe being unlocked.
I want to say that in these 3 other examples, we are generally using
"old_state" to refer to "the state immediately prior to locally mutating
it," which can sometimes mean a locked and sometimes unlocked state. To me,
that's why the usage in LWLockRelease feels a little confusing, since it is
referring to "the state after we mutate it, but corresponding to an older
semantic state."
> But the name "newstate" wouldn't be great, either.
> I don't have a great name in mind, so perhaps a comment instead?
Hmm, maybe "unlocked_state" would be a better name in LWLockRelease, when
considered across these examples? Alternatively a comment like "Calculate
the "old" state, that is, prior to lock acquisition."
Regards,
Jacob
