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

Commiteed959a

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 parent3838fa2 commiteed959a

File tree

3 files changed

+37
-34
lines changed

3 files changed

+37
-34
lines changed

‎src/backend/lib/dshash.c

Lines changed: 10 additions & 34 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. */
@@ -194,6 +192,10 @@ static inline bool equal_keys(dshash_table *hash_table,
194192
#definePARTITION_LOCK(hash_table,i)\
195193
(&(hash_table)->control->partitions[(i)].lock)
196194

195+
#defineASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table) \
196+
Assert(!LWLockAnyHeldByMe(&(hash_table)->control->partitions[0].lock, \
197+
DSHASH_NUM_PARTITIONS, sizeof(dshash_partition)))
198+
197199
/*
198200
* Create a new hash table backed by the given dynamic shared area, with the
199201
* given parameters. The returned object is allocated in backend-local memory
@@ -234,9 +236,6 @@ dshash_create(dsa_area *area, const dshash_parameters *params, void *arg)
234236
}
235237
}
236238

237-
hash_table->find_locked= false;
238-
hash_table->find_exclusively_locked= false;
239-
240239
/*
241240
* Set up the initial array of buckets. Our initial size is the same as
242241
* the number of partitions.
@@ -285,8 +284,6 @@ dshash_attach(dsa_area *area, const dshash_parameters *params,
285284
hash_table->params=*params;
286285
hash_table->arg=arg;
287286
hash_table->control=dsa_get_address(area,control);
288-
hash_table->find_locked= false;
289-
hash_table->find_exclusively_locked= false;
290287
Assert(hash_table->control->magic==DSHASH_MAGIC);
291288

292289
/*
@@ -309,7 +306,7 @@ dshash_attach(dsa_area *area, const dshash_parameters *params,
309306
void
310307
dshash_detach(dshash_table*hash_table)
311308
{
312-
Assert(!hash_table->find_locked);
309+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
313310

314311
/* The hash table may have been destroyed. Just free local memory. */
315312
pfree(hash_table);
@@ -400,7 +397,7 @@ dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
400397
partition=PARTITION_FOR_HASH(hash);
401398

402399
Assert(hash_table->control->magic==DSHASH_MAGIC);
403-
Assert(!hash_table->find_locked);
400+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
404401

405402
LWLockAcquire(PARTITION_LOCK(hash_table,partition),
406403
exclusive ?LW_EXCLUSIVE :LW_SHARED);
@@ -418,8 +415,6 @@ dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
418415
else
419416
{
420417
/* The caller will free the lock by calling dshash_release_lock. */
421-
hash_table->find_locked= true;
422-
hash_table->find_exclusively_locked=exclusive;
423418
returnENTRY_FROM_ITEM(item);
424419
}
425420
}
@@ -449,7 +444,7 @@ dshash_find_or_insert(dshash_table *hash_table,
449444
partition=&hash_table->control->partitions[partition_index];
450445

451446
Assert(hash_table->control->magic==DSHASH_MAGIC);
452-
Assert(!hash_table->find_locked);
447+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
453448

454449
restart:
455450
LWLockAcquire(PARTITION_LOCK(hash_table,partition_index),
@@ -494,8 +489,6 @@ dshash_find_or_insert(dshash_table *hash_table,
494489
}
495490

496491
/* The caller must release the lock with dshash_release_lock. */
497-
hash_table->find_locked= true;
498-
hash_table->find_exclusively_locked= true;
499492
returnENTRY_FROM_ITEM(item);
500493
}
501494

@@ -514,7 +507,7 @@ dshash_delete_key(dshash_table *hash_table, const void *key)
514507
boolfound;
515508

516509
Assert(hash_table->control->magic==DSHASH_MAGIC);
517-
Assert(!hash_table->find_locked);
510+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
518511

519512
hash=hash_key(hash_table,key);
520513
partition=PARTITION_FOR_HASH(hash);
@@ -551,14 +544,10 @@ dshash_delete_entry(dshash_table *hash_table, void *entry)
551544
size_tpartition=PARTITION_FOR_HASH(item->hash);
552545

553546
Assert(hash_table->control->magic==DSHASH_MAGIC);
554-
Assert(hash_table->find_locked);
555-
Assert(hash_table->find_exclusively_locked);
556547
Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table,partition),
557548
LW_EXCLUSIVE));
558549

559550
delete_item(hash_table,item);
560-
hash_table->find_locked= false;
561-
hash_table->find_exclusively_locked= false;
562551
LWLockRelease(PARTITION_LOCK(hash_table,partition));
563552
}
564553

@@ -572,13 +561,7 @@ dshash_release_lock(dshash_table *hash_table, void *entry)
572561
size_tpartition_index=PARTITION_FOR_HASH(item->hash);
573562

574563
Assert(hash_table->control->magic==DSHASH_MAGIC);
575-
Assert(hash_table->find_locked);
576-
Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table,partition_index),
577-
hash_table->find_exclusively_locked
578-
?LW_EXCLUSIVE :LW_SHARED));
579564

580-
hash_table->find_locked= false;
581-
hash_table->find_exclusively_locked= false;
582565
LWLockRelease(PARTITION_LOCK(hash_table,partition_index));
583566
}
584567

@@ -644,7 +627,7 @@ dshash_seq_next(dshash_seq_status *status)
644627
if (status->curpartition==-1)
645628
{
646629
Assert(status->curbucket==0);
647-
Assert(!status->hash_table->find_locked);
630+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(status->hash_table);
648631

649632
status->curpartition=0;
650633

@@ -702,8 +685,6 @@ dshash_seq_next(dshash_seq_status *status)
702685

703686
status->curitem=
704687
dsa_get_address(status->hash_table->area,next_item_pointer);
705-
status->hash_table->find_locked= true;
706-
status->hash_table->find_exclusively_locked=status->exclusive;
707688

708689
/*
709690
* The caller may delete the item. Store the next item in case of
@@ -722,9 +703,6 @@ dshash_seq_next(dshash_seq_status *status)
722703
void
723704
dshash_seq_term(dshash_seq_status*status)
724705
{
725-
status->hash_table->find_locked= false;
726-
status->hash_table->find_exclusively_locked= false;
727-
728706
if (status->curpartition >=0)
729707
LWLockRelease(PARTITION_LOCK(status->hash_table,status->curpartition));
730708
}
@@ -743,8 +721,6 @@ dshash_delete_current(dshash_seq_status *status)
743721

744722
Assert(status->exclusive);
745723
Assert(hash_table->control->magic==DSHASH_MAGIC);
746-
Assert(hash_table->find_locked);
747-
Assert(hash_table->find_exclusively_locked);
748724
Assert(LWLockHeldByMeInMode(PARTITION_LOCK(hash_table,partition),
749725
LW_EXCLUSIVE));
750726

@@ -762,7 +738,7 @@ dshash_dump(dshash_table *hash_table)
762738
size_tj;
763739

764740
Assert(hash_table->control->magic==DSHASH_MAGIC);
765-
Assert(!hash_table->find_locked);
741+
ASSERT_NO_PARTITION_LOCKS_HELD_BY_ME(hash_table);
766742

767743
for (i=0;i<DSHASH_NUM_PARTITIONS;++i)
768744
{

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,6 +1925,32 @@ LWLockHeldByMe(LWLock *l)
19251925
return false;
19261926
}
19271927

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

‎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