Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit4f88dba

Browse files
committed
Fix lock assertions in dshash.c.
dshash.c previously maintained flags to be able to assert that youdidn't hold any partition lock. These flags could get out of sync withreality in error scenarios.Get rid of all that, and make assertions about the locks themselvesinstead. Since LWLockHeldByMe() loops internally, we don't want to putthat inside another loop over all partition locks. Introduce a newdebugging-only interface LWLockAnyHeldByMe() to avoid that.This problem was noted by Tom and Andres while reviewing changes tosupport the new shared memory stats system, and later showed up inreality while working on commit389869a.Back-patch to 11, where dshash.c arrived.Reported-by: Tom Lane <tgl@sss.pgh.pa.us>Reported-by: Andres Freund <andres@anarazel.de>Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>Reviewed-by: Zhihong Yu <zyu@yugabyte.com>Reviewed-by: Andres Freund <andres@anarazel.de>Discussion:https://postgr.es/m/20220311012712.botrpsikaufzteyt@alap3.anarazel.deDiscussion:https://postgr.es/m/CA%2BhUKGJ31Wce6HJ7xnVTKWjFUWQZPBngxfJVx4q0E98pDr3kAw%40mail.gmail.com
1 parent09224a3 commit4f88dba

File tree

3 files changed

+37
-27
lines changed

3 files changed

+37
-27
lines changed

‎src/backend/lib/dshash.c

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,6 @@ struct dshash_table
110110
dshash_table_control*control;/* Control object in DSM. */
111111
dsa_pointer*buckets;/* Current bucket pointers in DSM. */
112112
size_tsize_log2;/* log2(number of buckets) */
113-
boolfind_locked;/* Is any partition lock held by 'find'? */
114-
boolfind_exclusively_locked;/* ... exclusively? */
115113
};
116114

117115
/* Given a pointer to an item, find the entry (user data) it holds. */
@@ -186,6 +184,10 @@ static inline bool equal_keys(dshash_table *hash_table,
186184
#definePARTITION_LOCK(hash_table,i)\
187185
(&(hash_table)->control->partitions[(i)].lock)
188186

187+
#defineASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table) \
188+
Assert(!LWLockAnyHeldByMe(&(hash_table)->control->partitions[0].lock, \
189+
DSHASH_NUM_PARTITIONS, sizeof(dshash_partition)))
190+
189191
/*
190192
* Create a new hash table backed by the given dynamic shared area, with the
191193
* given parameters. The returned object is allocated in backend-local memory
@@ -226,9 +228,6 @@ dshash_create(dsa_area *area, const dshash_parameters *params, void *arg)
226228
}
227229
}
228230

229-
hash_table->find_locked= false;
230-
hash_table->find_exclusively_locked= false;
231-
232231
/*
233232
* Set up the initial array of buckets. Our initial size is the same as
234233
* the number of partitions.
@@ -277,8 +276,6 @@ dshash_attach(dsa_area *area, const dshash_parameters *params,
277276
hash_table->params=*params;
278277
hash_table->arg=arg;
279278
hash_table->control=dsa_get_address(area,control);
280-
hash_table->find_locked= false;
281-
hash_table->find_exclusively_locked= false;
282279
Assert(hash_table->control->magic==DSHASH_MAGIC);
283280

284281
/*
@@ -301,7 +298,7 @@ dshash_attach(dsa_area *area, const dshash_parameters *params,
301298
void
302299
dshash_detach(dshash_table*hash_table)
303300
{
304-
Assert(!hash_table->find_locked);
301+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
305302

306303
/* The hash table may have been destroyed. Just free local memory. */
307304
pfree(hash_table);
@@ -392,7 +389,7 @@ dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
392389
partition=PARTITION_FOR_HASH(hash);
393390

394391
Assert(hash_table->control->magic==DSHASH_MAGIC);
395-
Assert(!hash_table->find_locked);
392+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
396393

397394
LWLockAcquire(PARTITION_LOCK(hash_table,partition),
398395
exclusive ?LW_EXCLUSIVE :LW_SHARED);
@@ -409,9 +406,7 @@ dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
409406
}
410407
else
411408
{
412-
/* The caller will free the lock by calling dshash_release. */
413-
hash_table->find_locked= true;
414-
hash_table->find_exclusively_locked=exclusive;
409+
/* The caller will free the lock by calling dshash_release_lock. */
415410
returnENTRY_FROM_ITEM(item);
416411
}
417412
}
@@ -441,7 +436,7 @@ dshash_find_or_insert(dshash_table *hash_table,
441436
partition=&hash_table->control->partitions[partition_index];
442437

443438
Assert(hash_table->control->magic==DSHASH_MAGIC);
444-
Assert(!hash_table->find_locked);
439+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
445440

446441
restart:
447442
LWLockAcquire(PARTITION_LOCK(hash_table,partition_index),
@@ -486,8 +481,6 @@ dshash_find_or_insert(dshash_table *hash_table,
486481
}
487482

488483
/* The caller must release the lock with dshash_release_lock. */
489-
hash_table->find_locked= true;
490-
hash_table->find_exclusively_locked= true;
491484
returnENTRY_FROM_ITEM(item);
492485
}
493486

@@ -506,7 +499,7 @@ dshash_delete_key(dshash_table *hash_table, const void *key)
506499
boolfound;
507500

508501
Assert(hash_table->control->magic==DSHASH_MAGIC);
509-
Assert(!hash_table->find_locked);
502+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
510503

511504
hash=hash_key(hash_table,key);
512505
partition=PARTITION_FOR_HASH(hash);
@@ -543,14 +536,10 @@ dshash_delete_entry(dshash_table *hash_table, void *entry)
543536
size_tpartition=PARTITION_FOR_HASH(item->hash);
544537

545538
Assert(hash_table->control->magic==DSHASH_MAGIC);
546-
Assert(hash_table->find_locked);
547-
Assert(hash_table->find_exclusively_locked);
548539
Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table,partition),
549540
LW_EXCLUSIVE));
550541

551542
delete_item(hash_table,item);
552-
hash_table->find_locked= false;
553-
hash_table->find_exclusively_locked= false;
554543
LWLockRelease(PARTITION_LOCK(hash_table,partition));
555544
}
556545

@@ -564,13 +553,7 @@ dshash_release_lock(dshash_table *hash_table, void *entry)
564553
size_tpartition_index=PARTITION_FOR_HASH(item->hash);
565554

566555
Assert(hash_table->control->magic==DSHASH_MAGIC);
567-
Assert(hash_table->find_locked);
568-
Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table,partition_index),
569-
hash_table->find_exclusively_locked
570-
?LW_EXCLUSIVE :LW_SHARED));
571556

572-
hash_table->find_locked= false;
573-
hash_table->find_exclusively_locked= false;
574557
LWLockRelease(PARTITION_LOCK(hash_table,partition_index));
575558
}
576559

@@ -603,7 +586,7 @@ dshash_dump(dshash_table *hash_table)
603586
size_tj;
604587

605588
Assert(hash_table->control->magic==DSHASH_MAGIC);
606-
Assert(!hash_table->find_locked);
589+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
607590

608591
for (i=0;i<DSHASH_NUM_PARTITIONS;++i)
609592
{

‎src/backend/storage/lmgr/lwlock.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1843,6 +1843,32 @@ LWLockHeldByMe(LWLock *l)
18431843
return false;
18441844
}
18451845

1846+
/*
1847+
* LWLockHeldByMe - test whether my process holds any of an array of locks
1848+
*
1849+
* This is meant as debug support only.
1850+
*/
1851+
bool
1852+
LWLockAnyHeldByMe(LWLock*l,intnlocks,size_tstride)
1853+
{
1854+
char*held_lock_addr;
1855+
char*begin;
1856+
char*end;
1857+
inti;
1858+
1859+
begin= (char*)l;
1860+
end=begin+nlocks*stride;
1861+
for (i=0;i<num_held_lwlocks;i++)
1862+
{
1863+
held_lock_addr= (char*)held_lwlocks[i].lock;
1864+
if (held_lock_addr >=begin&&
1865+
held_lock_addr<end&&
1866+
(held_lock_addr-begin) %stride==0)
1867+
return true;
1868+
}
1869+
return false;
1870+
}
1871+
18461872
/*
18471873
* LWLockHeldByMeInMode - test whether my process holds a lock in given mode
18481874
*

‎src/include/storage/lwlock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ extern void LWLockRelease(LWLock *lock);
150150
externvoidLWLockReleaseClearVar(LWLock*lock,uint64*valptr,uint64val);
151151
externvoidLWLockReleaseAll(void);
152152
externboolLWLockHeldByMe(LWLock*lock);
153+
externboolLWLockAnyHeldByMe(LWLock*lock,intnlocks,size_tstride);
153154
externboolLWLockHeldByMeInMode(LWLock*lock,LWLockModemode);
154155

155156
externboolLWLockWaitForVar(LWLock*lock,uint64*valptr,uint64oldval,uint64*newval);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp