10000 Rework locking code in GetMultiXactIdMembers · postgres/postgres@a0b808b · GitHub
[go: up one dir, main page]
Skip to content

Commit a0b808b

Browse files
alvherrealvherre
committed
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 f9baaf9 commit a0b808b

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)
0