On Mon, Apr 11, 2016 at 12:01 AM, Andres Freund <and...@anarazel.de> wrote:
>> InitBufferPool() manually fixes up alignment; that should probably be
>> removed now.
Attached patch does that.
>> I also wonder if it doesn't make sense to fix PG_CACHE_LINE_SIZE to
>> 64byte on x86. I personally think a manual ifdef in pg_config_manual.h
>> is ok for that.
>
> Also, this doesn't seem to be complete. This now aligns sizes to
> cacheline boundaries, but it doesn't actually align the returned values
> afaics? That might kind of work sometimes, if freeoffset is initially
> aligned to PG_CACHE_LINE_SIZE, but that's not guaranteed, it's just
> shmhdr->freeoffset += MAXALIGN(sizeof(slock_t));
And tries to fix that.
> Additionally, doesn't this obsolete
>
> /*
> * Preferred alignment for disk I/O buffers. On some CPUs, copies between
> * user space and kernel space are significantly faster if the user buffer
> * is aligned on a larger-than-MAXALIGN boundary. Ideally this should be
> * a platform-dependent value, but for now we just hard-wire it.
> */
> #define ALIGNOF_BUFFER 32
I didn't go as far as trying to remove this; a few other random things
are using it.
> and
>
> /* extra alignment for large requests, since they are probably
> buffers */
> if (size >= BLCKSZ)
> newStart = BUFFERALIGN(newStart);
But I got this one.
I fixed the InitBufferPool issue you mentioned in the other email, too.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c
index a5cffc7..5804870 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -76,11 +76,9 @@ InitBufferPool(void)
/* Align descriptors to a cacheline boundary. */
BufferDescriptors = (BufferDescPadded *)
- CACHELINEALIGN(
- ShmemInitStruct("Buffer Descriptors",
- NBuffers * sizeof(BufferDescPadded)
- + PG_CACHE_LINE_SIZE,
- &foundDescs));
+ ShmemInitStruct("Buffer Descriptors",
+ NBuffers * sizeof(BufferDescPadded),
+ &foundDescs);
BufferBlocks = (char *)
ShmemInitStruct("Buffer Blocks",
@@ -88,10 +86,9 @@ InitBufferPool(void)
/* Align lwlocks to cacheline boundary */
BufferIOLWLockArray = (LWLockMinimallyPadded *)
- CACHELINEALIGN(ShmemInitStruct("Buffer IO Locks",
- NBuffers * (Size) sizeof(LWLockMinimallyPadded)
- + PG_CACHE_LINE_SIZE,
- &foundIOLocks));
+ ShmemInitStruct("Buffer IO Locks",
+ NBuffers * (Size) sizeof(LWLockMinimallyPadded),
+ &foundIOLocks);
BufferIOLWLockTranche.name = "buffer_io";
BufferIOLWLockTranche.array_base = BufferIOLWLockArray;
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 1ad68cd..8e6cbdb 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -112,6 +112,7 @@ void
InitShmemAllocation(void)
{
PGShmemHeader *shmhdr = ShmemSegHdr;
+ char *aligned;
Assert(shmhdr != NULL);
@@ -139,6 +140,11 @@ InitShmemAllocation(void)
shmhdr->freeoffset += MAXALIGN(sizeof(slock_t));
Assert(shmhdr->freeoffset <= shmhdr->totalsize);
+ /* Make sure the first allocation begins on a cache line boundary. */
+ aligned = (char *)
+ (CACHELINEALIGN((((char *) shmhdr) + shmhdr->freeoffset)));
+ shmhdr->freeoffset = aligned - (char *) shmhdr;
+
SpinLockInit(ShmemLock);
/* ShmemIndex can't be set up yet (need LWLocks first) */
@@ -189,10 +195,6 @@ ShmemAlloc(Size size)
newStart = ShmemSegHdr->freeoffset;
- /* extra alignment for large requests, since they are probably buffers */
- if (size >= BLCKSZ)
- newStart = BUFFERALIGN(newStart);
-
newFree = newStart + size;
if (newFree <= ShmemSegHdr->totalsize)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
