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/