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

Commit10b9af3

Browse files
committed
Avoid using potentially-under-aligned page buffers.
There's a project policy against using plain "char buf[BLCKSZ]" localor static variables as page buffers; preferred style is to palloc ormalloc each buffer to ensure it is MAXALIGN'd. However, that policy'sbeen ignored in an increasing number of places. We've apparently gotaway with it so far, probably because (a) relatively few people useplatforms on which misalignment causes core dumps and/or (b) thevariables chance to be sufficiently aligned anyway. But this is notsomething to rely on. Moreover, even if we don't get a core dump,we might be paying a lot of cycles for misaligned accesses.To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlockthat the compiler must allocate with sufficient alignment, and usethose in place of plain char arrays.I used these types even for variables where there's no risk of amisaligned access, since ensuring proper alignment should makekernel data transfers faster. I also changed some places wherewe had been palloc'ing short-lived buffers, for coding styleuniformity and to save palloc/pfree overhead.Since this seems to be a live portability hazard (despite the lackof field reports), back-patch to all supported versions.Patch by me; thanks to Michael Paquier for review.Discussion:https://postgr.es/m/1535618100.1286.3.camel@credativ.de
1 parent1664c8b commit10b9af3

File tree

21 files changed

+129
-153
lines changed

21 files changed

+129
-153
lines changed

‎contrib/bloom/blinsert.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ typedef struct
3636
int64indtuples;/* total number of tuples indexed */
3737
MemoryContexttmpCtx;/* temporary memory context reset after each
3838
* tuple */
39-
chardata[BLCKSZ];/* cached page */
39+
PGAlignedBlockdata;/* cached page */
4040
intcount;/* number of tuples in cached page */
4141
}BloomBuildState;
4242

@@ -52,7 +52,7 @@ flushCachedPage(Relation index, BloomBuildState *buildstate)
5252

5353
state=GenericXLogStart(index);
5454
page=GenericXLogRegisterBuffer(state,buffer,GENERIC_XLOG_FULL_IMAGE);
55-
memcpy(page,buildstate->data,BLCKSZ);
55+
memcpy(page,buildstate->data.data,BLCKSZ);
5656
GenericXLogFinish(state);
5757
UnlockReleaseBuffer(buffer);
5858
}
@@ -63,8 +63,8 @@ flushCachedPage(Relation index, BloomBuildState *buildstate)
6363
staticvoid
6464
initCachedPage(BloomBuildState*buildstate)
6565
{
66-
memset(buildstate->data,0,BLCKSZ);
67-
BloomInitPage(buildstate->data,0);
66+
memset(buildstate->data.data,0,BLCKSZ);
67+
BloomInitPage(buildstate->data.data,0);
6868
buildstate->count=0;
6969
}
7070

@@ -84,7 +84,7 @@ bloomBuildCallback(Relation index, HeapTuple htup, Datum *values,
8484
itup=BloomFormTuple(&buildstate->blstate,&htup->t_self,values,isnull);
8585

8686
/* Try to add next item to cached page */
87-
if (BloomPageAddItem(&buildstate->blstate,buildstate->data,itup))
87+
if (BloomPageAddItem(&buildstate->blstate,buildstate->data.data,itup))
8888
{
8989
/* Next item was added successfully */
9090
buildstate->count++;
@@ -98,7 +98,7 @@ bloomBuildCallback(Relation index, HeapTuple htup, Datum *values,
9898

9999
initCachedPage(buildstate);
100100

101-
if (!BloomPageAddItem(&buildstate->blstate,buildstate->data,itup))
101+
if (!BloomPageAddItem(&buildstate->blstate,buildstate->data.data,itup))
102102
{
103103
/* We shouldn't be here since we're inserting to the empty page */
104104
elog(ERROR,"could not add new bloom tuple to empty page");

‎contrib/pg_prewarm/pg_prewarm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ typedef enum
3737
PREWARM_BUFFER
3838
}PrewarmType;
3939

40-
staticcharblockbuffer[BLCKSZ];
40+
staticPGAlignedBlockblockbuffer;
4141

4242
/*
4343
* pg_prewarm(regclass, mode text, fork text,
@@ -179,7 +179,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
179179
for (block=first_block;block <=last_block;++block)
180180
{
181181
CHECK_FOR_INTERRUPTS();
182-
smgrread(rel->rd_smgr,forkNumber,block,blockbuffer);
182+
smgrread(rel->rd_smgr,forkNumber,block,blockbuffer.data);
183183
++blocks_done;
184184
}
185185
}

‎src/backend/access/gin/ginentrypage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
616616
Pagelpage=PageGetTempPageCopy(BufferGetPage(origbuf));
617617
Pagerpage=PageGetTempPageCopy(BufferGetPage(origbuf));
618618
SizepageSize=PageGetPageSize(lpage);
619-
chartupstore[2*BLCKSZ];
619+
PGAlignedBlocktupstore[2];/* could need 2 pages' worth of tuples */
620620

621621
entryPreparePage(btree,lpage,off,insertData,updateblkno);
622622

@@ -625,7 +625,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
625625
* one after another in a temporary workspace.
626626
*/
627627
maxoff=PageGetMaxOffsetNumber(lpage);
628-
ptr=tupstore;
628+
ptr=tupstore[0].data;
629629
for (i=FirstOffsetNumber;i <=maxoff;i++)
630630
{
631631
if (i==off)
@@ -658,7 +658,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf,
658658
GinInitPage(rpage,GinPageGetOpaque(lpage)->flags,pageSize);
659659
GinInitPage(lpage,GinPageGetOpaque(rpage)->flags,pageSize);
660660

661-
ptr=tupstore;
661+
ptr=tupstore[0].data;
662662
maxoff++;
663663
lsize=0;
664664

‎src/backend/access/gin/ginfast.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,15 @@ writeListPage(Relation index, Buffer buffer,
6363
size=0;
6464
OffsetNumberl,
6565
off;
66-
char*workspace;
66+
PGAlignedBlockworkspace;
6767
char*ptr;
6868

69-
/* workspace could be a local array; we use palloc for alignment */
70-
workspace=palloc(BLCKSZ);
71-
7269
START_CRIT_SECTION();
7370

7471
GinInitBuffer(buffer,GIN_LIST);
7572

7673
off=FirstOffsetNumber;
77-
ptr=workspace;
74+
ptr=workspace.data;
7875

7976
for (i=0;i<ntuples;i++)
8077
{
@@ -126,7 +123,7 @@ writeListPage(Relation index, Buffer buffer,
126123
XLogRegisterData((char*)&data,sizeof(ginxlogInsertListPage));
127124

128125
XLogRegisterBuffer(0,buffer,REGBUF_WILL_INIT);
129-
XLogRegisterBufData(0,workspace,size);
126+
XLogRegisterBufData(0,workspace.data,size);
130127

131128
recptr=XLogInsert(RM_GIN_ID,XLOG_GIN_INSERT_LISTPAGE);
132129
PageSetLSN(page,recptr);
@@ -139,8 +136,6 @@ writeListPage(Relation index, Buffer buffer,
139136

140137
END_CRIT_SECTION();
141138

142-
pfree(workspace);
143-
144139
returnfreesize;
145140
}
146141

‎src/backend/access/hash/hashpage.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,7 @@ static bool
998998
_hash_alloc_buckets(Relationrel,BlockNumberfirstblock,uint32nblocks)
999999
{
10001000
BlockNumberlastblock;
1001-
charzerobuf[BLCKSZ];
1001+
PGAlignedBlockzerobuf;
10021002
Pagepage;
10031003
HashPageOpaqueovflopaque;
10041004

@@ -1011,7 +1011,7 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
10111011
if (lastblock<firstblock||lastblock==InvalidBlockNumber)
10121012
return false;
10131013

1014-
page= (Page)zerobuf;
1014+
page= (Page)zerobuf.data;
10151015

10161016
/*
10171017
* Initialize the page. Just zeroing the page won't work; see
@@ -1032,11 +1032,11 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
10321032
log_newpage(&rel->rd_node,
10331033
MAIN_FORKNUM,
10341034
lastblock,
1035-
zerobuf,
1035+
zerobuf.data,
10361036
true);
10371037

10381038
RelationOpenSmgr(rel);
1039-
smgrextend(rel->rd_smgr,MAIN_FORKNUM,lastblock,zerobuf, false);
1039+
smgrextend(rel->rd_smgr,MAIN_FORKNUM,lastblock,zerobuf.data, false);
10401040

10411041
return true;
10421042
}

‎src/backend/access/heap/heapam.c

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2662,7 +2662,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
26622662
HeapTuple*heaptuples;
26632663
inti;
26642664
intndone;
2665-
char*scratch=NULL;
2665+
PGAlignedBlockscratch;
26662666
Pagepage;
26672667
boolneedwal;
26682668
SizesaveFreeSpace;
@@ -2679,14 +2679,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
26792679
heaptuples[i]=heap_prepare_insert(relation,tuples[i],
26802680
xid,cid,options);
26812681

2682-
/*
2683-
* Allocate some memory to use for constructing the WAL record. Using
2684-
* palloc() within a critical section is not safe, so we allocate this
2685-
* beforehand.
2686-
*/
2687-
if (needwal)
2688-
scratch=palloc(BLCKSZ);
2689-
26902682
/*
26912683
* We're about to do the actual inserts -- but check for conflict first,
26922684
* to minimize the possibility of having to roll back work we've just
@@ -2779,7 +2771,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
27792771
uint8info=XLOG_HEAP2_MULTI_INSERT;
27802772
char*tupledata;
27812773
inttotaldatalen;
2782-
char*scratchptr=scratch;
2774+
char*scratchptr=scratch.data;
27832775
boolinit;
27842776
intbufflags=0;
27852777

@@ -2838,7 +2830,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
28382830
scratchptr+=datalen;
28392831
}
28402832
totaldatalen=scratchptr-tupledata;
2841-
Assert((scratchptr-scratch)<BLCKSZ);
2833+
Assert((scratchptr-scratch.data)<BLCKSZ);
28422834

28432835
if (need_tuple_data)
28442836
xlrec->flags |=XLH_INSERT_CONTAINS_NEW_TUPLE;
@@ -2865,7 +2857,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
28652857
bufflags |=REGBUF_KEEP_DATA;
28662858

28672859
XLogBeginInsert();
2868-
XLogRegisterData((char*)xlrec,tupledata-scratch);
2860+
XLogRegisterData((char*)xlrec,tupledata-scratch.data);
28692861
XLogRegisterBuffer(0,buffer,REGBUF_STANDARD |bufflags);
28702862

28712863
XLogRegisterBufData(0,tupledata,totaldatalen);

‎src/backend/access/heap/visibilitymap.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -645,10 +645,9 @@ static void
645645
vm_extend(Relationrel,BlockNumbervm_nblocks)
646646
{
647647
BlockNumbervm_nblocks_now;
648-
Pagepg;
648+
PGAlignedBlockpg;
649649

650-
pg= (Page)palloc(BLCKSZ);
651-
PageInit(pg,BLCKSZ,0);
650+
PageInit((Page)pg.data,BLCKSZ,0);
652651

653652
/*
654653
* We use the relation extension lock to lock out other backends trying to
@@ -679,10 +678,10 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
679678
/* Now extend the file */
680679
while (vm_nblocks_now<vm_nblocks)
681680
{
682-
PageSetChecksumInplace(pg,vm_nblocks_now);
681+
PageSetChecksumInplace((Page)pg.data,vm_nblocks_now);
683682

684683
smgrextend(rel->rd_smgr,VISIBILITYMAP_FORKNUM,vm_nblocks_now,
685-
(char*)pg, false);
684+
pg.data, false);
686685
vm_nblocks_now++;
687686
}
688687

@@ -699,6 +698,4 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
699698
rel->rd_smgr->smgr_vm_nblocks=vm_nblocks_now;
700699

701700
UnlockRelationForExtension(rel,ExclusiveLock);
702-
703-
pfree(pg);
704701
}

‎src/backend/access/transam/generic_xlog.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,11 @@ typedef struct
6161
/* State of generic xlog record construction */
6262
structGenericXLogState
6363
{
64-
/*
65-
* page's images. Should be first in this struct to have MAXALIGN'ed
66-
* images addresses, because some code working with pages directly aligns
67-
* addresses, not offsets from beginning of page
68-
*/
69-
charimages[MAX_GENERIC_XLOG_PAGES*BLCKSZ];
64+
/* Info about each page, see above */
7065
PageDatapages[MAX_GENERIC_XLOG_PAGES];
7166
boolisLogged;
67+
/* Page images (properly aligned) */
68+
PGAlignedBlockimages[MAX_GENERIC_XLOG_PAGES];
7269
};
7370

7471
staticvoidwriteFragment(PageData*pageData,OffsetNumberoffset,
@@ -251,12 +248,12 @@ computeDelta(PageData *pageData, Page curpage, Page targetpage)
251248
#ifdefWAL_DEBUG
252249
if (XLOG_DEBUG)
253250
{
254-
chartmp[BLCKSZ];
251+
PGAlignedBlocktmp;
255252

256-
memcpy(tmp,curpage,BLCKSZ);
257-
applyPageRedo(tmp,pageData->delta,pageData->deltaLen);
258-
if (memcmp(tmp,targetpage,targetLower)!=0||
259-
memcmp(tmp+targetUpper,targetpage+targetUpper,
253+
memcpy(tmp.data,curpage,BLCKSZ);
254+
applyPageRedo(tmp.data,pageData->delta,pageData->deltaLen);
255+
if (memcmp(tmp.data,targetpage,targetLower)!=0||
256+
memcmp(tmp.data+targetUpper,targetpage+targetUpper,
260257
BLCKSZ-targetUpper)!=0)
261258
elog(ERROR,"result of generic xlog apply does not match");
262259
}
@@ -277,7 +274,7 @@ GenericXLogStart(Relation relation)
277274

278275
for (i=0;i<MAX_GENERIC_XLOG_PAGES;i++)
279276
{
280-
state->pages[i].image=state->images+BLCKSZ*i;
277+
state->pages[i].image=state->images[i].data;
281278
state->pages[i].buffer=InvalidBuffer;
282279
}
283280

‎src/backend/access/transam/xlog.c

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3182,8 +3182,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
31823182
{
31833183
charpath[MAXPGPATH];
31843184
chartmppath[MAXPGPATH];
3185-
charzbuffer_raw[XLOG_BLCKSZ+MAXIMUM_ALIGNOF];
3186-
char*zbuffer;
3185+
PGAlignedXLogBlockzbuffer;
31873186
XLogSegNoinstalled_segno;
31883187
XLogSegNomax_segno;
31893188
intfd;
@@ -3237,17 +3236,13 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock)
32373236
* fsync below) that all the indirect blocks are down on disk. Therefore,
32383237
* fdatasync(2) or O_DSYNC will be sufficient to sync future writes to the
32393238
* log file.
3240-
*
3241-
* Note: ensure the buffer is reasonably well-aligned; this may save a few
3242-
* cycles transferring data to the kernel.
32433239
*/
3244-
zbuffer= (char*)MAXALIGN(zbuffer_raw);
3245-
memset(zbuffer,0,XLOG_BLCKSZ);
3240+
memset(zbuffer.data,0,XLOG_BLCKSZ);
32463241
for (nbytes=0;nbytes<XLogSegSize;nbytes+=XLOG_BLCKSZ)
32473242
{
32483243
errno=0;
32493244
pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_WRITE);
3250-
if ((int)write(fd,zbuffer,XLOG_BLCKSZ)!= (int)XLOG_BLCKSZ)
3245+
if ((int)write(fd,zbuffer.data,XLOG_BLCKSZ)!= (int)XLOG_BLCKSZ)
32513246
{
32523247
intsave_errno=errno;
32533248

@@ -3355,7 +3350,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
33553350
{
33563351
charpath[MAXPGPATH];
33573352
chartmppath[MAXPGPATH];
3358-
charbuffer[XLOG_BLCKSZ];
3353+
PGAlignedXLogBlockbuffer;
33593354
intsrcfd;
33603355
intfd;
33613356
intnbytes;
@@ -3399,15 +3394,15 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
33993394
* zeros.
34003395
*/
34013396
if (nread<sizeof(buffer))
3402-
memset(buffer,0,sizeof(buffer));
3397+
memset(buffer.data,0,sizeof(buffer));
34033398

34043399
if (nread>0)
34053400
{
34063401
if (nread>sizeof(buffer))
34073402
nread=sizeof(buffer);
34083403
errno=0;
34093404
pgstat_report_wait_start(WAIT_EVENT_WAL_COPY_READ);
3410-
if (read(srcfd,buffer,nread)!=nread)
3405+
if (read(srcfd,buffer.data,nread)!=nread)
34113406
{
34123407
if (errno!=0)
34133408
ereport(ERROR,
@@ -3423,7 +3418,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno,
34233418
}
34243419
errno=0;
34253420
pgstat_report_wait_start(WAIT_EVENT_WAL_COPY_WRITE);
3426-
if ((int)write(fd,buffer,sizeof(buffer))!= (int)sizeof(buffer))
3421+
if ((int)write(fd,buffer.data,sizeof(buffer))!= (int)sizeof(buffer))
34273422
{
34283423
intsave_errno=errno;
34293424

@@ -6410,8 +6405,11 @@ StartupXLOG(void)
64106405
xlogreader->system_identifier=ControlFile->system_identifier;
64116406

64126407
/*
6413-
* Allocate pages dedicated to WAL consistency checks, those had better be
6414-
* aligned.
6408+
* Allocate two page buffers dedicated to WAL consistency checks. We do
6409+
* it this way, rather than just making static arrays, for two reasons:
6410+
* (1) no need to waste the storage in most instantiations of the backend;
6411+
* (2) a static char array isn't guaranteed to have any particular
6412+
* alignment, whereas palloc() will provide MAXALIGN'd storage.
64156413
*/
64166414
replay_image_masked= (char*)palloc(BLCKSZ);
64176415
master_image_masked= (char*)palloc(BLCKSZ);

0 commit comments

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