Thread: ShmemAlloc() alignment patch
In ShmemAlloc() we have: newStart = BUFFERALIGN(newStart); newSpace = (void *) (ShmemBase + newStart); return newSpace; Notice that though newStart is ALIGNOF_BUFFER, ShmemBase is not. Thus the newSpace is not aligned as we disired. Attached please find the patch. Regards, Qingqing Index: src/backend/storage/ipc/shmem.c =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/storage/ipc/shmem.c,v retrieving revision 1.92 diff -c -r1.92 shmem.c *** src/backend/storage/ipc/shmem.c 13 Jul 2006 16:49:16 -0000 1.92 --- src/backend/storage/ipc/shmem.c 14 Jul 2006 06:47:43 -0000 *************** *** 169,178 **** SpinLockAcquire(ShmemLock); 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) --- 169,179 ---- SpinLockAcquire(ShmemLock); newStart = shmemseghdr->freeoffset; + newSpace = (void *) MAKE_PTR(newStart); /* extra alignment for large requests, since they are probably buffers */ if (size >= BLCKSZ) ! newStart = MAKE_OFFSET(BUFFERALIGN(newSpace)); newFree = newStart + size; if (newFree <= shmemseghdr->totalsize)
On Fri, Jul 14, 2006 at 02:50:31PM +0800, Qingqing Zhou wrote: > In ShmemAlloc() we have: > > newStart = BUFFERALIGN(newStart); > newSpace = (void *) (ShmemBase + newStart); > return newSpace; > > Notice that though newStart is ALIGNOF_BUFFER, ShmemBase is not. Thus the > newSpace is not aligned as we disired. How can ShmemBase not be aligned? Surely it's page-aligned? Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Martijn van Oosterhout <kleptog@svana.org> writes: > On Fri, Jul 14, 2006 at 02:50:31PM +0800, Qingqing Zhou wrote: >> Notice that though newStart is ALIGNOF_BUFFER, ShmemBase is not. Thus the >> newSpace is not aligned as we disired. > How can ShmemBase not be aligned? Surely it's page-aligned? That's certainly what the code expects. I'm disinclined to apply this patch unless you can identify a real system where ShmemBase might not point to a page boundary. (Note: in a standalone backend, the "shared memory segment" is just a huge malloc chunk, and so depending on your platform it might not be page-aligned. I don't feel a need to add cycles to ShmemAlloc to optimize this case, though. We only care about performance in the normal shared-memory case.) regards, tom lane
Tom Lane wrote: > Martijn van Oosterhout <kleptog@svana.org> writes: > > On Fri, Jul 14, 2006 at 02:50:31PM +0800, Qingqing Zhou wrote: > >> Notice that though newStart is ALIGNOF_BUFFER, ShmemBase is not. Thus the > >> newSpace is not aligned as we disired. > > > How can ShmemBase not be aligned? Surely it's page-aligned? > > That's certainly what the code expects. I'm disinclined to apply this > patch unless you can identify a real system where ShmemBase might not > point to a page boundary. > > (Note: in a standalone backend, the "shared memory segment" is just a > huge malloc chunk, and so depending on your platform it might not be > page-aligned. I don't feel a need to add cycles to ShmemAlloc to > optimize this case, though. We only care about performance in the > normal shared-memory case.) Should we add an assert? -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes: >>> How can ShmemBase not be aligned? Surely it's page-aligned? > Should we add an assert? No, because even if it's not page-aligned, there's no correctness issue here. (Besides, how would you know what the page size is on any given platform?) regards, tom lane