Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit fb9f6a9

Browse files
alvherrepull[bot]
authored andcommitted
Rework locking code in GetMultiXactIdMembers
After commit 53c2a97, the code flow around the "retry" goto label in GetMultiXactIdMembers was confused about what was possible: we never return there with a held lock, so there's no point in testing for one. This realization lets us simplify the code a bit. While at it, make the scope of a couple of local variables in the same function a bit tighter. Per Coverity.
1 parent 6f72d8a commit fb9f6a9

File tree

1 file changed

+22
-31
lines changed

1 file changed

+22
-31
lines changed

src/backend/access/transam/multixact.c

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,14 +1247,12 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
12471247
MultiXactOffset offset;
12481248
int length;
12491249
int truelength;
1250-
int i;
12511250
MultiXactId oldestMXact;
12521251
MultiXactId nextMXact;
12531252
MultiXactId tmpMXact;
12541253
MultiXactOffset nextOffset;
12551254
MultiXactMember *ptr;
12561255
LWLock *lock;
1257-
LWLock *prevlock = NULL;
12581256

12591257
debug_elog3(DEBUG2, "GetMembers: asked for %u", multi);
12601258

@@ -1361,18 +1359,9 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
13611359
pageno = MultiXactIdToOffsetPage(multi);
13621360
entryno = MultiXactIdToOffsetEntry(multi);
13631361

1364-
/*
1365-
* If this page falls under a different bank, release the old bank's lock
1366-
* and acquire the lock of the new bank.
1367-
*/
1362+
/* Acquire the bank lock for the page we need. */
13681363
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
1369-
if (lock != prevlock)
1370-
{
1371-
if (prevlock != NULL)
1372-
LWLockRelease(prevlock);
1373-
LWLockAcquire(lock, LW_EXCLUSIVE);
1374-
prevlock = lock;
1375-
}
1364+
LWLockAcquire(lock, LW_EXCLUSIVE);
13761365

13771366
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, multi);
13781367
offptr = (MultiXactOffset *) MultiXactOffsetCtl->shared->page_buffer[slotno];
@@ -1407,17 +1396,19 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
14071396

14081397
if (pageno != prev_pageno)
14091398
{
1399+
LWLock *newlock;
1400+
14101401
/*
14111402
* Since we're going to access a different SLRU page, if this page
14121403
* falls under a different bank, release the old bank's lock and
14131404
* acquire the lock of the new bank.
14141405
*/
1415-
lock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
1416-
if (prevlock != lock)
1406+
newlock = SimpleLruGetBankLock(MultiXactOffsetCtl, pageno);
1407+
if (newlock != lock)
14171408
{
1418-
LWLockRelease(prevlock);
1419-
LWLockAcquire(lock, LW_EXCLUSIVE);
1420-
prevlock = lock;
1409+
LWLockRelease(lock);
1410+
LWLockAcquire(newlock, LW_EXCLUSIVE);
1411+
lock = newlock;
14211412
}
14221413
slotno = SimpleLruReadPage(MultiXactOffsetCtl, pageno, true, tmpMXact);
14231414
}
@@ -1429,8 +1420,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
14291420
if (nextMXOffset == 0)
14301421
{
14311422
/* Corner case 2: next multixact is still being filled in */
1432-
LWLockRelease(prevlock);
1433-
prevlock = NULL;
1423+
LWLockRelease(lock);
14341424
CHECK_FOR_INTERRUPTS();
14351425
pg_usleep(1000L);
14361426
goto retry;
@@ -1439,14 +1429,14 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
14391429
length = nextMXOffset - offset;
14401430
}
14411431

1442-
LWLockRelease(prevlock);
1443-
prevlock = NULL;
1432+
LWLockRelease(lock);
1433+
lock = NULL;
14441434

14451435
ptr = (MultiXactMember *) palloc(length * sizeof(MultiXactMember));
14461436

14471437
truelength = 0;
14481438
prev_pageno = -1;
1449-
for (i = 0; i < length; i++, offset++)
1439+
for (int i = 0; i < length; i++, offset++)
14501440
{
14511441
TransactionId *xactptr;
14521442
uint32 *flagsptr;
@@ -1459,18 +1449,20 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
14591449

14601450
if (pageno != prev_pageno)
14611451
{
1452+
LWLock *newlock;
1453+
14621454
/*
14631455
* Since we're going to access a different SLRU page, if this page
14641456
* falls under a different bank, release the old bank's lock and
14651457
* acquire the lock of the new bank.
14661458
*/
1467-
lock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
1468-
if (lock != prevlock)
1459+
newlock = SimpleLruGetBankLock(MultiXactMemberCtl, pageno);
1460+
if (newlock != lock)
14691461
{
1470-
if (prevlock)
1471-
LWLockRelease(prevlock);
1472-
LWLockAcquire(lock, LW_EXCLUSIVE);
1473-
prevlock = lock;
1462+
if (lock)
1463+
LWLockRelease(lock);
1464+
LWLockAcquire(newlock, LW_EXCLUSIVE);
1465+
lock = newlock;
14741466
}
14751467

14761468
slotno = SimpleLruReadPage(MultiXactMemberCtl, pageno, true, multi);
@@ -1496,8 +1488,7 @@ GetMultiXactIdMembers(MultiXactId multi, MultiXactMember **members,
14961488
truelength++;
14971489
}
14981490

1499-
if (prevlock)
1500-
LWLockRelease(prevlock);
1491+
LWLockRelease(lock);
15011492

15021493
/* A multixid with zero members should not happen */
15031494
Assert(truelength > 0);

0 commit comments

Comments
 (0)