Re: pgsql: Improve performance of subsystems on top of SLRU

On 2024-Feb-28, Alvaro Herrera wrote:

> Improve performance of subsystems on top of SLRU

Coverity had the following complaint about this commit:

________________________________________________________________________________________________________
*** CID NNNNNNN: Control flow issues (DEADCODE)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/transam/multixact.c: 1375 in GetMultiXactIdMembers()
1369 * and acquire the lock of the new bank.
1370 */
1371 lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
1372 if (lock != prevlock)
1373 {
1374 if (prevlock != NULL)
>>> CID 1592913: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "LWLockRelease(prevlock);".
1375 LWLockRelease(prevlock);
1376 LWLockAcquire(lock, LW_EXCLUSIVE);
1377 prevlock = lock;
1378 }
1379
1380 slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);

And I think it's correct that this is somewhat bogus, or at least
confusing: the only way to have control back here on line 1371 after
having executed once is via the "goto retry" line below; and there we
release "prevlock" and set it to NULL beforehand, so it's impossible for
prevlock to be NULL. Looking closer I think this code is all confused,
so I suggest to rework it as shown in the attached patch.

I'll have a look at the other places where we use this "prevlock" coding
pattern tomorrow.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/