]> granicus.if.org Git - postgresql/commitdiff
Avoid using potentially-under-aligned page buffers.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 1 Sep 2018 19:27:13 +0000 (15:27 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 1 Sep 2018 19:27:13 +0000 (15:27 -0400)
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

17 files changed:
contrib/pg_prewarm/pg_prewarm.c
src/backend/access/gin/ginentrypage.c
src/backend/access/gin/ginfast.c
src/backend/access/hash/hashpage.c
src/backend/access/heap/heapam.c
src/backend/access/heap/visibilitymap.c
src/backend/access/transam/xlog.c
src/backend/access/transam/xloginsert.c
src/backend/access/transam/xlogreader.c
src/backend/commands/tablecmds.c
src/backend/replication/walsender.c
src/backend/storage/file/buffile.c
src/backend/storage/freespace/freespace.c
src/bin/pg_basebackup/receivelog.c
src/bin/pg_resetxlog/pg_resetxlog.c
src/bin/pg_rewind/copy_fetch.c
src/include/c.h

index d81f93bba7951cd3c87070d2b13bb50367d26d25..2c7aaa2da8d1393097ee2d08dac9b3a600f4baaf 100644 (file)
@@ -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;
                }
        }
index a022f50ffa6dd342f14207fc17d7451a89953c3f..db0112f5ac00cc9fbb478d8de5190bc6ec892a8d 100644 (file)
@@ -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;
 
index 65471eb9f23bd6e4512aa18ee3f98d120f7a3fa7..bdfd45ea5459dd8808c93040c8463ba9450cff1b 100644 (file)
@@ -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;
 }
 
index 3c11cce72ab9d2af0c8462c24ed0e68200f6e054..bdb002f25196449bfc8d6b71a8db7e891f8e4135 100644 (file)
@@ -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;
 }
index c70bcc7213b1a8c2d71a6836cef4b32674293d0e..83a03f3d0af48f4b67981e2f9781ea7acdb937bc 100644 (file)
@@ -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);
index b9828d8b5b9b753d4c8b8f0345fabd3f81deb983..129b5d3d727709787724dec311e38fde1765b608 100644 (file)
@@ -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);
 }
index 630f9ca142e96a7bca0135a584c483a2a1289b24..0e95b3cef81de5d39f953184141c3c0216cbdce8 100644 (file)
@@ -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;
 
index dbc906c38a665ec79dfabede0e30acc2d0b2dbea..49795b48a9de3af7925fbcc4398f0fbee7bc18ea 100644 (file)
@@ -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);
        }
index 630f6915ae4fd65dc5c2092e52836bf153014c15..5a6cdae0e6b938910dd5ea45c004e2fa04dd5c5c 100644 (file)
@@ -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 */
index 06cf286e9bd22e0dae159a1bd8e4c61f3b0d4557..572477f7c2137a0454195e70c1a986a30d3baf74 100644 (file)
@@ -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
index 9174a22ae9d396a8bd4f090b7ec0981d16d6afd6..534f052c319b7ac85cbdfbad1be96dd46df62843 100644 (file)
@@ -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);
index ea4d689179f83ccdd3d9ca396ac944e7f2f2fe98..9a14addfd7a908ea27c62069c03a62197a7ce09e 100644 (file)
@@ -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;
index 135d76788eb32ca541e0c5192e1709b618686e11..f96d7b093acbc666e1d652665f565afab94d1229 100644 (file)
@@ -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);
 }
 
 /*
index fb70b204f79a735ec176cafd9844e97d322ef701..e0036fbe7c6509268f3d8ee0e7d84b4eb6fe37ae 100644 (file)
@@ -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)
        {
index 74eed8879d989cde27308005def91604514d5c6c..3826f959da19cf3440b6faf3d5d17211a6e54580 100644 (file)
@@ -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;
index 964c541060078c768420a8a30d7771f33e4dc8a8..6799fec5cc3ccf311cc72a91f60d7748f310fcc4 100644 (file)
@@ -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;
        }
 
index 7168113c56ee463559c43b2e9a8bdcecc1430bc8..2481ded8385810a3ca4e35b814a30c002f922f70 100644 (file)
@@ -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)