From: | Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, John Gorman <johngorman2(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PATCH: two slab-like memory allocators |
Date: | 2017-02-28 00:44:42 |
Message-ID: | c7c1bb85-d1d0-05a5-4d4c-3047d6f053e7@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 02/27/2017 06:42 PM, Andres Freund wrote:
> On 2017-02-27 12:27:48 -0500, Tom Lane wrote:
>> Andres Freund <andres(at)anarazel(dot)de> writes:
>>> The best theory I have so far that I have is that slab.c's idea of
>>> StandardChunkHeader's size doesn't match what mcxt.c think it is
>>> (because slab.c simply embeds StandardChunkHeader, but mcxt uses
>>> MAXALIGN(sizeof(StandardChunkHeader))). That's not good, but I don't
>>> quite see how that'd cause the issue, since StandardChunkHeader's size
>>> should always be properly sized.
>>
>> Uh, wrong. On a 32-bit machine with debug enabled, StandardChunkHeader
>> will contain 3 4-byte fields. However, there are some such machines on
>> which MAXALIGN is 8. For example, looking at termite's configure
>> output:
>>
>> checking size of void *... 4
>> checking size of size_t... 4
>> checking size of long... 4
>> checking alignment of short... 2
>> checking alignment of int... 4
>> checking alignment of long... 4
>> checking alignment of long long int... 8
>> checking alignment of double... 8
>>
>> axolotl's output looks similar. I expect my old HPPA dinosaur
>> will show the failure as well.
>
> Yea, I hadn't yet realized when writing that that termite actually,
> despite running on ppc64, compiles a 32bit postgres. Will thus
> duplicate StandardChunkHeader's contents in to slab.c :( - I don't
> see an easy way around that...
>
I've tried this - essentially copying the StandardChunkHeader's contents
into SlabChunk, but that does not seem to do the trick, sadly. Per
pahole, the structures then (at least on armv7l) look like this:
struct SlabChunk {
void * block; /* 0 4 */
MemoryContext context; /* 4 4 */
Size size; /* 8 4 */
Size requested_size; /* 12 4 */
/* size: 16, cachelines: 1, members: 4 */
/* last cacheline: 16 bytes */
};
struct StandardChunkHeader {
MemoryContext context; /* 0 4 */
Size size; /* 4 4 */
Size requested_size; /* 8 4 */
/* size: 12, cachelines: 1, members: 3 */
/* last cacheline: 12 bytes */
};
So SlabChunk happens to be perfectly aligned (MAXIMUM_ALIGNOF=8), and so
pfree() grabs the block pointer but thinks it's the context :-(
Not sure what to do about this - the only thing I can think about is
splitting SlabChunk into two separate structures, and align them
independently.
The attached patch does that - it probably needs a bit more work on the
comments to make it commit-ready, but it fixes the test_deconding tests
on the rpi3 board I'm using for testing.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment | Content-Type | Size |
---|---|---|
slab-fix.patch | binary/octet-stream | 7.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2017-02-28 00:50:37 | Re: PATCH: Make pg_stop_backup() archive wait optional |
Previous Message | David Steele | 2017-02-28 00:42:55 | Re: PATCH: Make pg_stop_backup() archive wait optional |