From: Tom Lane Date: Sat, 1 Sep 2018 19:27:13 +0000 (-0400) Subject: Avoid using potentially-under-aligned page buffers. X-Git-Tag: REL9_5_15~69 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=03ffe5553c1d6238dc235e8d5e8f1295734e5c79;p=postgresql Avoid using potentially-under-aligned page buffers. There's a project policy against using plain "char buf[BLCKSZ]" local or static variables as page buffers; preferred style is to palloc or malloc each buffer to ensure it is MAXALIGN'd. However, that policy's been ignored in an increasing number of places. We've apparently got away with it so far, probably because (a) relatively few people use platforms on which misalignment causes core dumps and/or (b) the variables chance to be sufficiently aligned anyway. But this is not something 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 PGAlignedXLogBlock that the compiler must allocate with sufficient alignment, and use those in place of plain char arrays. I used these types even for variables where there's no risk of a misaligned access, since ensuring proper alignment should make kernel data transfers faster. I also changed some places where we had been palloc'ing short-lived buffers, for coding style uniformity and to save palloc/pfree overhead. Since this seems to be a live portability hazard (despite the lack of 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 --- diff --git a/contrib/pg_prewarm/pg_prewarm.c b/contrib/pg_prewarm/pg_prewarm.c index d81f93bba7..2c7aaa2da8 100644 --- a/contrib/pg_prewarm/pg_prewarm.c +++ b/contrib/pg_prewarm/pg_prewarm.c @@ -37,7 +37,7 @@ typedef enum PREWARM_BUFFER } PrewarmType; -static char blockbuffer[BLCKSZ]; +static PGAlignedBlock blockbuffer; /* * pg_prewarm(regclass, mode text, fork text, @@ -179,7 +179,7 @@ pg_prewarm(PG_FUNCTION_ARGS) for (block = first_block; block <= last_block; ++block) { CHECK_FOR_INTERRUPTS(); - smgrread(rel->rd_smgr, forkNumber, block, blockbuffer); + smgrread(rel->rd_smgr, forkNumber, block, blockbuffer.data); ++blocks_done; } } diff --git a/src/backend/access/gin/ginentrypage.c b/src/backend/access/gin/ginentrypage.c index a022f50ffa..db0112f5ac 100644 --- a/src/backend/access/gin/ginentrypage.c +++ b/src/backend/access/gin/ginentrypage.c @@ -615,7 +615,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf, Page lpage = PageGetTempPageCopy(BufferGetPage(origbuf)); Page rpage = PageGetTempPageCopy(BufferGetPage(origbuf)); Size pageSize = PageGetPageSize(lpage); - char tupstore[2 * BLCKSZ]; + PGAlignedBlock tupstore[2]; /* could need 2 pages' worth of tuples */ entryPreparePage(btree, lpage, off, insertData, updateblkno); @@ -624,7 +624,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf, * one after another in a temporary workspace. */ maxoff = PageGetMaxOffsetNumber(lpage); - ptr = tupstore; + ptr = tupstore[0].data; for (i = FirstOffsetNumber; i <= maxoff; i++) { if (i == off) @@ -657,7 +657,7 @@ entrySplitPage(GinBtree btree, Buffer origbuf, GinInitPage(rpage, GinPageGetOpaque(lpage)->flags, pageSize); GinInitPage(lpage, GinPageGetOpaque(rpage)->flags, pageSize); - ptr = tupstore; + ptr = tupstore[0].data; maxoff++; lsize = 0; diff --git a/src/backend/access/gin/ginfast.c b/src/backend/access/gin/ginfast.c index 65471eb9f2..bdfd45ea54 100644 --- a/src/backend/access/gin/ginfast.c +++ b/src/backend/access/gin/ginfast.c @@ -55,18 +55,15 @@ writeListPage(Relation index, Buffer buffer, size = 0; OffsetNumber l, off; - char *workspace; + PGAlignedBlock workspace; char *ptr; - /* workspace could be a local array; we use palloc for alignment */ - workspace = palloc(BLCKSZ); - START_CRIT_SECTION(); GinInitBuffer(buffer, GIN_LIST); off = FirstOffsetNumber; - ptr = workspace; + ptr = workspace.data; for (i = 0; i < ntuples; i++) { @@ -118,7 +115,7 @@ writeListPage(Relation index, Buffer buffer, XLogRegisterData((char *) &data, sizeof(ginxlogInsertListPage)); XLogRegisterBuffer(0, buffer, REGBUF_WILL_INIT); - XLogRegisterBufData(0, workspace, size); + XLogRegisterBufData(0, workspace.data, size); recptr = XLogInsert(RM_GIN_ID, XLOG_GIN_INSERT_LISTPAGE); PageSetLSN(page, recptr); @@ -131,8 +128,6 @@ writeListPage(Relation index, Buffer buffer, END_CRIT_SECTION(); - pfree(workspace); - return freesize; } diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index 3c11cce72a..bdb002f251 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -710,7 +710,7 @@ static bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks) { BlockNumber lastblock; - char zerobuf[BLCKSZ]; + PGAlignedBlock zerobuf; lastblock = firstblock + nblocks - 1; @@ -721,10 +721,10 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks) if (lastblock < firstblock || lastblock == InvalidBlockNumber) return false; - MemSet(zerobuf, 0, sizeof(zerobuf)); + MemSet(zerobuf.data, 0, sizeof(zerobuf)); RelationOpenSmgr(rel); - smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf, false); + smgrextend(rel->rd_smgr, MAIN_FORKNUM, lastblock, zerobuf.data, false); return true; } diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index c70bcc7213..83a03f3d0a 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2414,7 +2414,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, HeapTuple *heaptuples; int i; int ndone; - char *scratch = NULL; + PGAlignedBlock scratch; Page page; bool needwal; Size saveFreeSpace; @@ -2431,14 +2431,6 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, heaptuples[i] = heap_prepare_insert(relation, tuples[i], xid, cid, options); - /* - * Allocate some memory to use for constructing the WAL record. Using - * palloc() within a critical section is not safe, so we allocate this - * beforehand. - */ - if (needwal) - scratch = palloc(BLCKSZ); - /* * We're about to do the actual inserts -- but check for conflict first, * to minimize the possibility of having to roll back work we've just @@ -2531,7 +2523,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, uint8 info = XLOG_HEAP2_MULTI_INSERT; char *tupledata; int totaldatalen; - char *scratchptr = scratch; + char *scratchptr = scratch.data; bool init; int bufflags = 0; @@ -2590,7 +2582,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, scratchptr += datalen; } totaldatalen = scratchptr - tupledata; - Assert((scratchptr - scratch) < BLCKSZ); + Assert((scratchptr - scratch.data) < BLCKSZ); if (need_tuple_data) xlrec->flags |= XLH_INSERT_CONTAINS_NEW_TUPLE; @@ -2617,7 +2609,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples, bufflags |= REGBUF_KEEP_DATA; XLogBeginInsert(); - XLogRegisterData((char *) xlrec, tupledata - scratch); + XLogRegisterData((char *) xlrec, tupledata - scratch.data); XLogRegisterBuffer(0, buffer, REGBUF_STANDARD | bufflags); XLogRegisterBufData(0, tupledata, totaldatalen); diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c index b9828d8b5b..129b5d3d72 100644 --- a/src/backend/access/heap/visibilitymap.c +++ b/src/backend/access/heap/visibilitymap.c @@ -611,10 +611,9 @@ static void vm_extend(Relation rel, BlockNumber vm_nblocks) { BlockNumber vm_nblocks_now; - Page pg; + PGAlignedBlock pg; - pg = (Page) palloc(BLCKSZ); - PageInit(pg, BLCKSZ, 0); + PageInit((Page) pg.data, BLCKSZ, 0); /* * We use the relation extension lock to lock out other backends trying to @@ -645,10 +644,10 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) /* Now extend the file */ while (vm_nblocks_now < vm_nblocks) { - PageSetChecksumInplace(pg, vm_nblocks_now); + PageSetChecksumInplace((Page) pg.data, vm_nblocks_now); smgrextend(rel->rd_smgr, VISIBILITYMAP_FORKNUM, vm_nblocks_now, - (char *) pg, false); + pg.data, false); vm_nblocks_now++; } @@ -665,6 +664,4 @@ vm_extend(Relation rel, BlockNumber vm_nblocks) rel->rd_smgr->smgr_vm_nblocks = vm_nblocks_now; UnlockRelationForExtension(rel, ExclusiveLock); - - pfree(pg); } diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 630f9ca142..0e95b3cef8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2976,8 +2976,7 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock) { char path[MAXPGPATH]; char tmppath[MAXPGPATH]; - char zbuffer_raw[XLOG_BLCKSZ + MAXIMUM_ALIGNOF]; - char *zbuffer; + PGAlignedXLogBlock zbuffer; XLogSegNo installed_segno; XLogSegNo max_segno; int fd; @@ -3031,16 +3030,12 @@ XLogFileInit(XLogSegNo logsegno, bool *use_existent, bool use_lock) * fsync below) that all the indirect blocks are down on disk. Therefore, * fdatasync(2) or O_DSYNC will be sufficient to sync future writes to the * log file. - * - * Note: ensure the buffer is reasonably well-aligned; this may save a few - * cycles transferring data to the kernel. */ - zbuffer = (char *) MAXALIGN(zbuffer_raw); - memset(zbuffer, 0, XLOG_BLCKSZ); + memset(zbuffer.data, 0, XLOG_BLCKSZ); for (nbytes = 0; nbytes < XLogSegSize; nbytes += XLOG_BLCKSZ) { errno = 0; - if ((int) write(fd, zbuffer, XLOG_BLCKSZ) != (int) XLOG_BLCKSZ) + if ((int) write(fd, zbuffer.data, XLOG_BLCKSZ) != (int) XLOG_BLCKSZ) { int save_errno = errno; @@ -3145,7 +3140,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno, { char path[MAXPGPATH]; char tmppath[MAXPGPATH]; - char buffer[XLOG_BLCKSZ]; + PGAlignedXLogBlock buffer; int srcfd; int fd; int nbytes; @@ -3189,14 +3184,14 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno, * zeros. */ if (nread < sizeof(buffer)) - memset(buffer, 0, sizeof(buffer)); + memset(buffer.data, 0, sizeof(buffer)); if (nread > 0) { if (nread > sizeof(buffer)) nread = sizeof(buffer); errno = 0; - if (read(srcfd, buffer, nread) != nread) + if (read(srcfd, buffer.data, nread) != nread) { if (errno != 0) ereport(ERROR, @@ -3210,7 +3205,7 @@ XLogFileCopy(XLogSegNo destsegno, TimeLineID srcTLI, XLogSegNo srcsegno, } } errno = 0; - if ((int) write(fd, buffer, sizeof(buffer)) != (int) sizeof(buffer)) + if ((int) write(fd, buffer.data, sizeof(buffer)) != (int) sizeof(buffer)) { int save_errno = errno; diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index dbc906c38a..49795b48a9 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -774,12 +774,12 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length, int32 len; int32 extra_bytes = 0; char *source; - char tmp[BLCKSZ]; + PGAlignedBlock tmp; if (hole_length != 0) { /* must skip the hole */ - source = tmp; + source = tmp.data; memcpy(source, page, hole_offset); memcpy(source + hole_offset, page + (hole_offset + hole_length), @@ -882,7 +882,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) if (lsn <= RedoRecPtr) { int flags; - char copied_buffer[BLCKSZ]; + PGAlignedBlock copied_buffer; char *origdata = (char *) BufferGetBlock(buffer); RelFileNode rnode; ForkNumber forkno; @@ -900,11 +900,11 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) uint16 lower = ((PageHeader) page)->pd_lower; uint16 upper = ((PageHeader) page)->pd_upper; - memcpy(copied_buffer, origdata, lower); - memcpy(copied_buffer + upper, origdata + upper, BLCKSZ - upper); + memcpy(copied_buffer.data, origdata, lower); + memcpy(copied_buffer.data + upper, origdata + upper, BLCKSZ - upper); } else - memcpy(copied_buffer, origdata, BLCKSZ); + memcpy(copied_buffer.data, origdata, BLCKSZ); XLogBeginInsert(); @@ -913,7 +913,7 @@ XLogSaveBufferForHint(Buffer buffer, bool buffer_std) flags |= REGBUF_STANDARD; BufferGetTag(buffer, &rnode, &forkno, &blkno); - XLogRegisterBlock(0, &rnode, forkno, blkno, copied_buffer, flags); + XLogRegisterBlock(0, &rnode, forkno, blkno, copied_buffer.data, flags); recptr = XLogInsert(RM_XLOG_ID, XLOG_FPI_FOR_HINT); } diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 630f6915ae..5a6cdae0e6 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1375,7 +1375,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) { DecodedBkpBlock *bkpb; char *ptr; - char tmp[BLCKSZ]; + PGAlignedBlock tmp; if (!record->blocks[block_id].in_use) return false; @@ -1388,7 +1388,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) if (bkpb->bimg_info & BKPIMAGE_IS_COMPRESSED) { /* If a backup block image is compressed, decompress it */ - if (pglz_decompress(ptr, bkpb->bimg_len, tmp, + if (pglz_decompress(ptr, bkpb->bimg_len, tmp.data, BLCKSZ - bkpb->hole_length) < 0) { report_invalid_record(record, "invalid compressed image at %X/%X, block %d", @@ -1397,7 +1397,7 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) block_id); return false; } - ptr = tmp; + ptr = tmp.data; } /* generate page, taking into account hole if necessary */ diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 06cf286e9b..572477f7c2 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9971,21 +9971,14 @@ static void copy_relation_data(SMgrRelation src, SMgrRelation dst, ForkNumber forkNum, char relpersistence) { - char *buf; + PGAlignedBlock buf; Page page; bool use_wal; bool copying_initfork; BlockNumber nblocks; BlockNumber blkno; - /* - * palloc the buffer so that it's MAXALIGN'd. If it were just a local - * char[] array, the compiler might align it on any byte boundary, which - * can seriously hurt transfer speed to and from the kernel; not to - * mention possibly making log_newpage's accesses to the page header fail. - */ - buf = (char *) palloc(BLCKSZ); - page = (Page) buf; + page = (Page) buf.data; /* * The init fork for an unlogged relation in many respects has to be @@ -10009,7 +10002,7 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst, /* If we got a cancel signal during the copy of the data, quit */ CHECK_FOR_INTERRUPTS(); - smgrread(src, forkNum, blkno, buf); + smgrread(src, forkNum, blkno, buf.data); if (!PageIsVerified(page, blkno)) ereport(ERROR, @@ -10035,11 +10028,9 @@ copy_relation_data(SMgrRelation src, SMgrRelation dst, * rel, because there's no need for smgr to schedule an fsync for this * write; we'll do it ourselves below. */ - smgrextend(dst, forkNum, blkno, buf, true); + smgrextend(dst, forkNum, blkno, buf.data, true); } - pfree(buf); - /* * If the rel is WAL-logged, must fsync before commit. We use heap_sync * to ensure that the toast table gets fsync'd too. (For a temp or diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 9174a22ae9..534f052c31 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -495,16 +495,16 @@ SendTimeLineHistory(TimeLineHistoryCmd *cmd) bytesleft = histfilelen; while (bytesleft > 0) { - char rbuf[BLCKSZ]; + PGAlignedBlock rbuf; int nread; - nread = read(fd, rbuf, sizeof(rbuf)); + nread = read(fd, rbuf.data, sizeof(rbuf)); if (nread <= 0) ereport(ERROR, (errcode_for_file_access(), errmsg("could not read file \"%s\": %m", path))); - pq_sendbytes(&buf, rbuf, nread); + pq_sendbytes(&buf, rbuf.data, nread); bytesleft -= nread; } CloseTransientFile(fd); diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index ea4d689179..9a14addfd7 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -86,7 +86,7 @@ struct BufFile off_t curOffset; /* offset part of current pos */ int pos; /* next read/write position in buffer */ int nbytes; /* total # of valid bytes in buffer */ - char buffer[BLCKSZ]; + PGAlignedBlock buffer; }; static BufFile *makeBufFile(File firstfile); @@ -254,7 +254,7 @@ BufFileLoadBuffer(BufFile *file) /* * Read whatever we can get, up to a full bufferload. */ - file->nbytes = FileRead(thisfile, file->buffer, sizeof(file->buffer)); + file->nbytes = FileRead(thisfile, file->buffer.data, sizeof(file->buffer)); if (file->nbytes < 0) file->nbytes = 0; file->offsets[file->curFile] += file->nbytes; @@ -317,7 +317,7 @@ BufFileDumpBuffer(BufFile *file) return; /* seek failed, give up */ file->offsets[file->curFile] = file->curOffset; } - bytestowrite = FileWrite(thisfile, file->buffer + wpos, bytestowrite); + bytestowrite = FileWrite(thisfile, file->buffer.data + wpos, bytestowrite); if (bytestowrite <= 0) return; /* failed to write */ file->offsets[file->curFile] += bytestowrite; @@ -385,7 +385,7 @@ BufFileRead(BufFile *file, void *ptr, size_t size) nthistime = size; Assert(nthistime > 0); - memcpy(ptr, file->buffer + file->pos, nthistime); + memcpy(ptr, file->buffer.data + file->pos, nthistime); file->pos += nthistime; ptr = (void *) ((char *) ptr + nthistime); @@ -432,7 +432,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size) nthistime = size; Assert(nthistime > 0); - memcpy(file->buffer + file->pos, ptr, nthistime); + memcpy(file->buffer.data + file->pos, ptr, nthistime); file->dirty = true; file->pos += nthistime; diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index 135d76788e..f96d7b093a 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -587,10 +587,9 @@ static void fsm_extend(Relation rel, BlockNumber fsm_nblocks) { BlockNumber fsm_nblocks_now; - Page pg; + PGAlignedBlock pg; - pg = (Page) palloc(BLCKSZ); - PageInit(pg, BLCKSZ, 0); + PageInit((Page) pg.data, BLCKSZ, 0); /* * We use the relation extension lock to lock out other backends trying to @@ -620,10 +619,10 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks) while (fsm_nblocks_now < fsm_nblocks) { - PageSetChecksumInplace(pg, fsm_nblocks_now); + PageSetChecksumInplace((Page) pg.data, fsm_nblocks_now); smgrextend(rel->rd_smgr, FSM_FORKNUM, fsm_nblocks_now, - (char *) pg, false); + pg.data, false); fsm_nblocks_now++; } @@ -631,8 +630,6 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks) rel->rd_smgr->smgr_fsm_nblocks = fsm_nblocks_now; UnlockRelationForExtension(rel, ExclusiveLock); - - pfree(pg); } /* diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c index fb70b204f7..e0036fbe7c 100644 --- a/src/bin/pg_basebackup/receivelog.c +++ b/src/bin/pg_basebackup/receivelog.c @@ -108,7 +108,7 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir, int f; char fn[MAXPGPATH]; struct stat statbuf; - char *zerobuf; + PGAlignedXLogBlock zerobuf; int bytes; XLogSegNo segno; @@ -154,11 +154,11 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir, } /* New, empty, file. So pad it to 16Mb with zeroes */ - zerobuf = pg_malloc0(XLOG_BLCKSZ); + memset(zerobuf.data, 0, XLOG_BLCKSZ); for (bytes = 0; bytes < XLogSegSize; bytes += XLOG_BLCKSZ) { errno = 0; - if (write(f, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (write(f, zerobuf.data, XLOG_BLCKSZ) != XLOG_BLCKSZ) { /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) @@ -166,13 +166,11 @@ open_walfile(XLogRecPtr startpoint, uint32 timeline, char *basedir, fprintf(stderr, _("%s: could not pad transaction log file \"%s\": %s\n"), progname, fn, strerror(errno)); - free(zerobuf); close(f); unlink(fn); return false; } } - free(zerobuf); if (lseek(f, SEEK_SET, 0) != 0) { diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c index 74eed8879d..3826f959da 100644 --- a/src/bin/pg_resetxlog/pg_resetxlog.c +++ b/src/bin/pg_resetxlog/pg_resetxlog.c @@ -1133,7 +1133,7 @@ KillExistingArchiveStatus(void) static void WriteEmptyXLOG(void) { - char *buffer; + PGAlignedXLogBlock buffer; XLogPageHeader page; XLogLongPageHeader longpage; XLogRecord *record; @@ -1143,12 +1143,10 @@ WriteEmptyXLOG(void) int nbytes; char *recptr; - /* Use malloc() to ensure buffer is MAXALIGNED */ - buffer = (char *) pg_malloc(XLOG_BLCKSZ); - page = (XLogPageHeader) buffer; - memset(buffer, 0, XLOG_BLCKSZ); + memset(buffer.data, 0, XLOG_BLCKSZ); /* Set up the XLOG page header */ + page = (XLogPageHeader) buffer.data; page->xlp_magic = XLOG_PAGE_MAGIC; page->xlp_info = XLP_LONG_HEADER; page->xlp_tli = ControlFile.checkPointCopy.ThisTimeLineID; @@ -1194,7 +1192,7 @@ WriteEmptyXLOG(void) } errno = 0; - if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ) { /* if write didn't set errno, assume problem is no disk space */ if (errno == 0) @@ -1205,11 +1203,11 @@ WriteEmptyXLOG(void) } /* Fill the rest of the file with zeroes */ - memset(buffer, 0, XLOG_BLCKSZ); + memset(buffer.data, 0, XLOG_BLCKSZ); for (nbytes = XLOG_BLCKSZ; nbytes < XLogSegSize; nbytes += XLOG_BLCKSZ) { errno = 0; - if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ) + if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ) { if (errno == 0) errno = ENOSPC; diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c index 964c541060..6799fec5cc 100644 --- a/src/bin/pg_rewind/copy_fetch.c +++ b/src/bin/pg_rewind/copy_fetch.c @@ -160,7 +160,7 @@ recurse_dir(const char *datadir, const char *parentpath, static void rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc) { - char buf[BLCKSZ]; + PGAlignedBlock buf; char srcpath[MAXPGPATH]; int srcfd; @@ -186,7 +186,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc) else len = end - begin; - readlen = read(srcfd, buf, len); + readlen = read(srcfd, buf.data, len); if (readlen < 0) pg_fatal("could not read file \"%s\": %s\n", @@ -194,7 +194,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc) else if (readlen == 0) pg_fatal("unexpected EOF while reading file \"%s\"\n", srcpath); - write_target_range(buf, begin, readlen); + write_target_range(buf.data, begin, readlen); begin += readlen; } diff --git a/src/include/c.h b/src/include/c.h index 7168113c56..2481ded838 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -960,6 +960,32 @@ typedef NameData *Name; * ---------------------------------------------------------------- */ +/* + * Use this, not "char buf[BLCKSZ]", to declare a field or local variable + * holding a page buffer, if that page might be accessed as a page and not + * just a string of bytes. Otherwise the variable might be under-aligned, + * causing problems on alignment-picky hardware. (In some places, we use + * this to declare buffers even though we only pass them to read() and + * write(), because copying to/from aligned buffers is usually faster than + * using unaligned buffers.) We include both "double" and "int64" in the + * union to ensure that the compiler knows the value must be MAXALIGN'ed + * (cf. configure's computation of MAXIMUM_ALIGNOF). + */ +typedef union PGAlignedBlock +{ + char data[BLCKSZ]; + double force_align_d; + int64 force_align_i64; +} PGAlignedBlock; + +/* Same, but for an XLOG_BLCKSZ-sized buffer */ +typedef union PGAlignedXLogBlock +{ + char data[XLOG_BLCKSZ]; + double force_align_d; + int64 force_align_i64; +} PGAlignedXLogBlock; + /* msb for char */ #define HIGHBIT (0x80) #define IS_HIGHBIT_SET(ch) ((unsigned char)(ch) & HIGHBIT)