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

Commit3e60c6f

Browse files
committed
Code review for shm_toc.h/.c.
Declare the toc_nentry field as uint32 not Size. Since shm_toc_lookup()reads the field without any lock, it has to be atomically readable, andwe do not assume that for fields wider than 32 bits. Performance wouldbe impossibly bad for entry counts approaching 2^32 anyway, so there isno need to try to preserve maximum width here.This is probably an academic issue, because even if reading int64 isn'tatomic, the high order half would never change in practice. Still, it'sa coding rule violation, so let's fix it.Adjust some other not-terribly-well-chosen data types too, and copy-editsome comments. Make shm_toc_attach's Asserts consistent withshm_toc_create's.None of this looks to be a live bug, so no need for back-patch.Discussion:https://postgr.es/m/16984.1496679541@sss.pgh.pa.us
1 parent614350a commit3e60c6f

File tree

2 files changed

+30
-25
lines changed

2 files changed

+30
-25
lines changed

‎src/backend/storage/ipc/shm_toc.c

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2017, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* src/include/storage/shm_toc.c
9+
* src/backend/storage/ipc/shm_toc.c
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -20,16 +20,16 @@
2020
typedefstructshm_toc_entry
2121
{
2222
uint64key;/* Arbitrary identifier */
23-
uint64offset;/*Bytes offset */
23+
Sizeoffset;/*Offset, in bytes, from TOC start */
2424
}shm_toc_entry;
2525

2626
structshm_toc
2727
{
28-
uint64toc_magic;/* Magic numberfor this TOC */
28+
uint64toc_magic;/* Magic numberidentifying this TOC */
2929
slock_ttoc_mutex;/* Spinlock for mutual exclusion */
3030
Sizetoc_total_bytes;/* Bytes managed by this TOC */
3131
Sizetoc_allocated_bytes;/* Bytes allocated of those managed */
32-
Sizetoc_nentry;/* Number of entries in TOC */
32+
uint32toc_nentry;/* Number of entries in TOC */
3333
shm_toc_entrytoc_entry[FLEXIBLE_ARRAY_MEMBER];
3434
};
3535

@@ -53,7 +53,7 @@ shm_toc_create(uint64 magic, void *address, Size nbytes)
5353

5454
/*
5555
* Attach to an existing table of contents. If the magic number found at
56-
* the target address doesn't match our expectations,returns NULL.
56+
* the target address doesn't match our expectations,return NULL.
5757
*/
5858
externshm_toc*
5959
shm_toc_attach(uint64magic,void*address)
@@ -64,7 +64,7 @@ shm_toc_attach(uint64 magic, void *address)
6464
returnNULL;
6565

6666
Assert(toc->toc_total_bytes >=toc->toc_allocated_bytes);
67-
Assert(toc->toc_total_bytes >= offsetof(shm_toc,toc_entry));
67+
Assert(toc->toc_total_bytes> offsetof(shm_toc,toc_entry));
6868

6969
returntoc;
7070
}
@@ -76,7 +76,7 @@ shm_toc_attach(uint64 magic, void *address)
7676
* just a way of dividing a single physical shared memory segment into logical
7777
* chunks that may be used for different purposes.
7878
*
79-
* Weallocated backwards from the end of the segment, so that the TOC entries
79+
* Weallocate backwards from the end of the segment, so that the TOC entries
8080
* can grow forward from the start of the segment.
8181
*/
8282
externvoid*
@@ -140,7 +140,7 @@ shm_toc_freespace(shm_toc *toc)
140140
/*
141141
* Insert a TOC entry.
142142
*
143-
* The idea here is that process setting up the shared memory segment will
143+
* The idea here is thattheprocess setting up the shared memory segment will
144144
* register the addresses of data structures within the segment using this
145145
* function. Each data structure will be identified using a 64-bit key, which
146146
* is assumed to be a well-known or discoverable integer. Other processes
@@ -155,17 +155,17 @@ shm_toc_freespace(shm_toc *toc)
155155
* data structure here. But the real idea here is just to give someone mapping
156156
* a dynamic shared memory the ability to find the bare minimum number of
157157
* pointers that they need to bootstrap. If you're storing a lot of stuff in
158-
*here, you're doing it wrong.
158+
*the TOC, you're doing it wrong.
159159
*/
160160
void
161161
shm_toc_insert(shm_toc*toc,uint64key,void*address)
162162
{
163163
volatileshm_toc*vtoc=toc;
164-
uint64total_bytes;
165-
uint64allocated_bytes;
166-
uint64nentry;
167-
uint64toc_bytes;
168-
uint64offset;
164+
Sizetotal_bytes;
165+
Sizeallocated_bytes;
166+
Sizenentry;
167+
Sizetoc_bytes;
168+
Sizeoffset;
169169

170170
/* Relativize pointer. */
171171
Assert(address> (void*)toc);
@@ -181,7 +181,8 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
181181

182182
/* Check for memory exhaustion and overflow. */
183183
if (toc_bytes+sizeof(shm_toc_entry)>total_bytes||
184-
toc_bytes+sizeof(shm_toc_entry)<toc_bytes)
184+
toc_bytes+sizeof(shm_toc_entry)<toc_bytes||
185+
nentry >=PG_UINT32_MAX)
185186
{
186187
SpinLockRelease(&toc->toc_mutex);
187188
ereport(ERROR,
@@ -220,10 +221,13 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
220221
void*
221222
shm_toc_lookup(shm_toc*toc,uint64key,boolnoError)
222223
{
223-
uint64nentry;
224-
uint64i;
224+
uint32nentry;
225+
uint32i;
225226

226-
/* Read the number of entries before we examine any entry. */
227+
/*
228+
* Read the number of entries before we examine any entry. We assume that
229+
* reading a uint32 is atomic.
230+
*/
227231
nentry=toc->toc_nentry;
228232
pg_read_barrier();
229233

‎src/include/storage/shm_toc.h

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@
2222
#ifndefSHM_TOC_H
2323
#defineSHM_TOC_H
2424

25-
#include"storage/shmem.h"
25+
#include"storage/shmem.h"/* for add_size() */
2626

27-
structshm_toc;
27+
/* shm_toc is an opaque type known only within shm_toc.c */
2828
typedefstructshm_tocshm_toc;
2929

3030
externshm_toc*shm_toc_create(uint64magic,void*address,Sizenbytes);
@@ -36,7 +36,9 @@ extern void *shm_toc_lookup(shm_toc *toc, uint64 key, bool noError);
3636

3737
/*
3838
* Tools for estimating how large a chunk of shared memory will be needed
39-
* to store a TOC and its dependent objects.
39+
* to store a TOC and its dependent objects. Note: we don't really support
40+
* large numbers of keys, but it's convenient to declare number_of_keys
41+
* as a Size anyway.
4042
*/
4143
typedefstruct
4244
{
@@ -47,11 +49,10 @@ typedef struct
4749
#defineshm_toc_initialize_estimator(e) \
4850
((e)->space_for_chunks = 0, (e)->number_of_keys = 0)
4951
#defineshm_toc_estimate_chunk(e,sz) \
50-
((e)->space_for_chunks = add_size((e)->space_for_chunks, \
51-
BUFFERALIGN((sz))))
52+
((e)->space_for_chunks = add_size((e)->space_for_chunks, BUFFERALIGN(sz)))
5253
#defineshm_toc_estimate_keys(e,cnt) \
53-
((e)->number_of_keys = add_size((e)->number_of_keys,(cnt)))
54+
((e)->number_of_keys = add_size((e)->number_of_keys, cnt))
5455

55-
externSizeshm_toc_estimate(shm_toc_estimator*);
56+
externSizeshm_toc_estimate(shm_toc_estimator*e);
5657

5758
#endif/* SHM_TOC_H */

0 commit comments

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