Subject: Re: pgsql: Generational memory allocator
To: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>
Cc: Simon Riggs <simon@2ndquadrant.com>,
 pgsql-committers <pgsql-committers@postgresql.org>,
 Andrew Dunstan <andrew@dunslane.net>
References: <E1eHa4J-0006hI-Q8@gemulon.postgresql.org>
 <20171123001649.y2nf7422iay6cw3v@alap3.anarazel.de>
 <CANP8+jKoT=Pg5W_3FcKiDf+FMrJhFYacxt_g5E0=tkmb5s8M6g@mail.gmail.com>
 <20171123193936.fncfnar5wkpx56mw@alap3.anarazel.de>
 <21043.1511467602@sss.pgh.pa.us>
From: Tomas Vondra <tv@fuzzy.cz>
Message-ID: <c09b1f12-8042-1e77-1b3e-ca68a2b44167@fuzzy.cz>
Date: Thu, 23 Nov 2017 22:34:57 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <21043.1511467602@sss.pgh.pa.us>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 11/23/2017 09:06 PM, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2017-11-23 20:57:10 +1100, Simon Riggs wrote:
>>> On 23 November 2017 at 11:16, Andres Freund <andres@anarazel.de> wrote:
>>>> Looks like it's not quite valgrind clean:
>>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2017-11-22%2022%3A30%3A01
> 
>>> It doesn't report anything useful that would allow me to fix this.
> 
>> Uh. It reports a failure, and you can go from there. The error output
>> actually is in postmaster.log but for some reason the buildfarm code
>> didn't display that in this case.
> 

It may not be immediately obvious that the failure is due to valgrind, 
but otherwise I agree it's up to us to investigate.

> I think it's a legitimate complaint that postmaster.log wasn't captured
> in this failure, but that's a buildfarm script oversight and hardly
> Andres' fault.
> 

Are the valgrind errors really written to postmaster log? I'm assuming 
it failed because valgrind ran into an issue and killed the process.

> In any case, valgrind failures are generally easy enough to reproduce
> locally.
> 

Right.

> Meanwhile, over on
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snapper&dt=2017-11-23%2013%3A56%3A17
> 
> we have
> 
> ccache gcc-4.7 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security  -I../../../../src/include  -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -I/usr/include/libxml2  -I/usr/include/mit-krb5  -c -o generation.o generation.c
> generation.c: In function ‘GenerationContextCreate’:
> generation.c:206:7: error: static assertion failed: "padding calculation in GenerationChunk is wrong"
> make[4]: *** [generation.o] Error 1
> 
> Looks to me like GenerationChunk imagines that 3*sizeof(pointer)
> must be a maxaligned quantity, which is wrong on platforms where
> MAXALIGN is bigger than sizeof(pointer).
> 

Hmmm, I see. Presumably adding this to GenerationChunk (similarly to 
what we do in AllocChunkData) would address the issue:

#if MAXIMUM_ALIGNOF > 4 && SIZEOF_VOID_P == 4
	Size		padding;
#endif

but I have no way to verify that (no access to such machine). I wonder 
why SlabChunk doesn't need to do that (perhaps a comment explaining that 
would be appropriate?).

regards
Tomas