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

Commit44cac93

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 parent5e8d670 commit44cac93

File tree

24 files changed

+141
-168
lines changed

24 files changed

+141
-168
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
@@ -36,7 +36,7 @@ typedef enum
3636
PREWARM_BUFFER
3737
}PrewarmType;
3838

39-
staticcharblockbuffer[BLCKSZ];
39+
staticPGAlignedBlockblockbuffer;
4040

4141
/*
4242
* pg_prewarm(regclass, mode text, fork text,
@@ -178,7 +178,7 @@ pg_prewarm(PG_FUNCTION_ARGS)
178178
for (block=first_block;block <=last_block;++block)
179179
{
180180
CHECK_FOR_INTERRUPTS();
181-
smgrread(rel->rd_smgr,forkNumber,block,blockbuffer);
181+
smgrread(rel->rd_smgr,forkNumber,block,blockbuffer.data);
182182
++blocks_done;
183183
}
184184
}

‎contrib/pg_standby/pg_standby.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -401,24 +401,21 @@ SetWALSegSize(void)
401401
{
402402
boolret_val= false;
403403
intfd;
404-
405-
/* malloc this buffer to ensure sufficient alignment: */
406-
char*buf= (char*)pg_malloc(XLOG_BLCKSZ);
404+
PGAlignedXLogBlockbuf;
407405

408406
Assert(WalSegSz==-1);
409407

410408
if ((fd=open(WALFilePath,O_RDWR,0))<0)
411409
{
412410
fprintf(stderr,"%s: could not open WAL file \"%s\": %s\n",
413411
progname,WALFilePath,strerror(errno));
414-
pg_free(buf);
415412
return false;
416413
}
417414

418415
errno=0;
419-
if (read(fd,buf,XLOG_BLCKSZ)==XLOG_BLCKSZ)
416+
if (read(fd,buf.data,XLOG_BLCKSZ)==XLOG_BLCKSZ)
420417
{
421-
XLogLongPageHeaderlonghdr= (XLogLongPageHeader)buf;
418+
XLogLongPageHeaderlonghdr= (XLogLongPageHeader)buf.data;
422419

423420
WalSegSz=longhdr->xlp_seg_size;
424421

@@ -455,7 +452,6 @@ SetWALSegSize(void)
455452
fflush(stderr);
456453

457454
close(fd);
458-
pg_free(buf);
459455
returnret_val;
460456
}
461457

‎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
@@ -64,18 +64,15 @@ writeListPage(Relation index, Buffer buffer,
6464
size=0;
6565
OffsetNumberl,
6666
off;
67-
char*workspace;
67+
PGAlignedBlockworkspace;
6868
char*ptr;
6969

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

7572
GinInitBuffer(buffer,GIN_LIST);
7673

7774
off=FirstOffsetNumber;
78-
ptr=workspace;
75+
ptr=workspace.data;
7976

8077
for (i=0;i<ntuples;i++)
8178
{
@@ -127,7 +124,7 @@ writeListPage(Relation index, Buffer buffer,
127124
XLogRegisterData((char*)&data,sizeof(ginxlogInsertListPage));
128125

129126
XLogRegisterBuffer(0,buffer,REGBUF_WILL_INIT);
130-
XLogRegisterBufData(0,workspace,size);
127+
XLogRegisterBufData(0,workspace.data,size);
131128

132129
recptr=XLogInsert(RM_GIN_ID,XLOG_GIN_INSERT_LISTPAGE);
133130
PageSetLSN(page,recptr);
@@ -140,8 +137,6 @@ writeListPage(Relation index, Buffer buffer,
140137

141138
END_CRIT_SECTION();
142139

143-
pfree(workspace);
144-
145140
returnfreesize;
146141
}
147142

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,7 @@ static bool
10001000
_hash_alloc_buckets(Relationrel,BlockNumberfirstblock,uint32nblocks)
10011001
{
10021002
BlockNumberlastblock;
1003-
charzerobuf[BLCKSZ];
1003+
PGAlignedBlockzerobuf;
10041004
Pagepage;
10051005
HashPageOpaqueovflopaque;
10061006

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

1016-
page= (Page)zerobuf;
1016+
page= (Page)zerobuf.data;
10171017

10181018
/*
10191019
* Initialize the page. Just zeroing the page won't work; see
@@ -1034,11 +1034,11 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
10341034
log_newpage(&rel->rd_node,
10351035
MAIN_FORKNUM,
10361036
lastblock,
1037-
zerobuf,
1037+
zerobuf.data,
10381038
true);
10391039

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

10431043
return true;
10441044
}

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2709,7 +2709,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
27092709
HeapTuple*heaptuples;
27102710
inti;
27112711
intndone;
2712-
char*scratch=NULL;
2712+
PGAlignedBlockscratch;
27132713
Pagepage;
27142714
boolneedwal;
27152715
SizesaveFreeSpace;
@@ -2726,14 +2726,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
27262726
heaptuples[i]=heap_prepare_insert(relation,tuples[i],
27272727
xid,cid,options);
27282728

2729-
/*
2730-
* Allocate some memory to use for constructing the WAL record. Using
2731-
* palloc() within a critical section is not safe, so we allocate this
2732-
* beforehand.
2733-
*/
2734-
if (needwal)
2735-
scratch=palloc(BLCKSZ);
2736-
27372729
/*
27382730
* We're about to do the actual inserts -- but check for conflict first,
27392731
* to minimize the possibility of having to roll back work we've just
@@ -2826,7 +2818,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
28262818
uint8info=XLOG_HEAP2_MULTI_INSERT;
28272819
char*tupledata;
28282820
inttotaldatalen;
2829-
char*scratchptr=scratch;
2821+
char*scratchptr=scratch.data;
28302822
boolinit;
28312823
intbufflags=0;
28322824

@@ -2885,7 +2877,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
28852877
scratchptr+=datalen;
28862878
}
28872879
totaldatalen=scratchptr-tupledata;
2888-
Assert((scratchptr-scratch)<BLCKSZ);
2880+
Assert((scratchptr-scratch.data)<BLCKSZ);
28892881

28902882
if (need_tuple_data)
28912883
xlrec->flags |=XLH_INSERT_CONTAINS_NEW_TUPLE;
@@ -2912,7 +2904,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
29122904
bufflags |=REGBUF_KEEP_DATA;
29132905

29142906
XLogBeginInsert();
2915-
XLogRegisterData((char*)xlrec,tupledata-scratch);
2907+
XLogRegisterData((char*)xlrec,tupledata-scratch.data);
29162908
XLogRegisterBuffer(0,buffer,REGBUF_STANDARD |bufflags);
29172909

29182910
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

0 commit comments

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