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

Commit5e7608e

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 parentec5f1fe commit5e7608e

File tree

3 files changed

+36
-26
lines changed

3 files changed

+36
-26
lines changed

‎src/backend/lib/dshash.c

Lines changed: 9 additions & 26 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);
@@ -410,8 +407,6 @@ dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
410407
else
411408
{
412409
/* The caller will free the lock by calling dshash_release_lock. */
413-
hash_table->find_locked= true;
414-
hash_table->find_exclusively_locked=exclusive;
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
@@ -1932,6 +1932,32 @@ LWLockHeldByMe(LWLock *l)
19321932
return false;
19331933
}
19341934

1935+
/*
1936+
* LWLockHeldByMe - test whether my process holds any of an array of locks
1937+
*
1938+
* This is meant as debug support only.
1939+
*/
1940+
bool
1941+
LWLockAnyHeldByMe(LWLock*l,intnlocks,size_tstride)
1942+
{
1943+
char*held_lock_addr;
1944+
char*begin;
1945+
char*end;
1946+
inti;
1947+
1948+
begin= (char*)l;
1949+
end=begin+nlocks*stride;
1950+
for (i=0;i<num_held_lwlocks;i++)
1951+
{
1952+
held_lock_addr= (char*)held_lwlocks[i].lock;
1953+
if (held_lock_addr >=begin&&
1954+
held_lock_addr<end&&
1955+
(held_lock_addr-begin) %stride==0)
1956+
return true;
1957+
}
1958+
return false;
1959+
}
1960+
19351961
/*
19361962
* LWLockHeldByMeInMode - test whether my process holds a lock in given mode
19371963
*

‎src/include/storage/lwlock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ extern void LWLockRelease(LWLock *lock);
120120
externvoidLWLockReleaseClearVar(LWLock*lock,uint64*valptr,uint64val);
121121
externvoidLWLockReleaseAll(void);
122122
externboolLWLockHeldByMe(LWLock*lock);
123+
externboolLWLockAnyHeldByMe(LWLock*lock,intnlocks,size_tstride);
123124
externboolLWLockHeldByMeInMode(LWLock*lock,LWLockModemode);
124125

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

0 commit comments

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