Re: [HACKERS] PATCH: two slab-like memory allocators - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [HACKERS] PATCH: two slab-like memory allocators
Date
Msg-id c7c1bb85-d1d0-05a5-4d4c-3047d6f053e7@2ndquadrant.com
Whole thread Raw
In response to Re: [HACKERS] PATCH: two slab-like memory allocators  (Andres Freund <andres@anarazel.de>)
Responses Re: [HACKERS] PATCH: two slab-like memory allocators
List 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@anarazel.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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional
Next
From: Michael Paquier
Date:
Subject: Re: [HACKERS] PATCH: Make pg_stop_backup() archive wait optional