Horton

Thread

Commits

  1. Fix two valgrind issues in slab allocator.

  1. bug in SlabAlloc / VALGRIND_MAKE_MEM_DEFINED

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2017-04-04T20:42:39Z

    Hi,

Andres nagged to me about valgrind runs taking much longer since 
9fab40ad introduced the SlabContext into reorderbuffer.c. And by 
"longer" I mean hours instead of minutes.

After a bit of investigation I stumbled on this line in slab.c:

   VALGRIND_MAKE_MEM_DEFINED(chunk, SlabChunkGetPointer(chunk));

which is wrong, because the second parameter should be size and not 
another pointer. This essentially marks a lot of memory as defined, 
which results in the extreme test runtime.

Instead, the line should be:

   VALGRIND_MAKE_MEM_DEFINED(SlabChunkGetPointer(chunk), sizeof(int32));

Patch attached.

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  2. Re: bug in SlabAlloc / VALGRIND_MAKE_MEM_DEFINED

    Tomas Vondra <tomas.vondra@2ndquadrant.com> — 2017-04-04T21:23:30Z

    On 04/04/2017 10:42 PM, Tomas Vondra wrote:
> Hi,
>
> Andres nagged to me about valgrind runs taking much longer since
> 9fab40ad introduced the SlabContext into reorderbuffer.c. And by
> "longer" I mean hours instead of minutes.
>
> After a bit of investigation I stumbled on this line in slab.c:
>
>   VALGRIND_MAKE_MEM_DEFINED(chunk, SlabChunkGetPointer(chunk));
>
> which is wrong, because the second parameter should be size and not
> another pointer. This essentially marks a lot of memory as defined,
> which results in the extreme test runtime.
>
> Instead, the line should be:
>
>   VALGRIND_MAKE_MEM_DEFINED(SlabChunkGetPointer(chunk), sizeof(int32));
>
> Patch attached.
>

Turns out SlabCheck() was missing VALGRIND_MAKE_MEM_DEFINED, resulting 
in a valgrind failure with --enable-assert. Updated patch version 
attached, passing all tests in test_decoding.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  3. Re: bug in SlabAlloc / VALGRIND_MAKE_MEM_DEFINED

    Andres Freund <andres@anarazel.de> — 2017-04-04T21:46:28Z

    On 2017-04-04 23:23:30 +0200, Tomas Vondra wrote:
> On 04/04/2017 10:42 PM, Tomas Vondra wrote:
> > Hi,
> > 
> > Andres nagged to me about valgrind runs taking much longer since
> > 9fab40ad introduced the SlabContext into reorderbuffer.c. And by
> > "longer" I mean hours instead of minutes.
> > 
> > After a bit of investigation I stumbled on this line in slab.c:
> > 
> >   VALGRIND_MAKE_MEM_DEFINED(chunk, SlabChunkGetPointer(chunk));
> > 
> > which is wrong, because the second parameter should be size and not
> > another pointer. This essentially marks a lot of memory as defined,
> > which results in the extreme test runtime.
> > 
> > Instead, the line should be:
> > 
> >   VALGRIND_MAKE_MEM_DEFINED(SlabChunkGetPointer(chunk), sizeof(int32));
> > 
> > Patch attached.
> > 
> 
> Turns out SlabCheck() was missing VALGRIND_MAKE_MEM_DEFINED, resulting in a
> valgrind failure with --enable-assert. Updated patch version attached,
> passing all tests in test_decoding.

Pushed, and re-enabled TestDecoding on skink.