Re: SlabCheck leaks memory into TopMemoryContext

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tv(at)fuzzy(dot)cz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: SlabCheck leaks memory into TopMemoryContext
Date: 2020-01-16 17:04:32
Message-ID: 20200116170432.gkpxd5xxhyodekw7@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 16, 2020 at 11:43:34AM -0500, Tom Lane wrote:
>Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> writes:
>> The attached fix should do the trick - it pre-allocates the space when
>> creating the context. There is a bit of complexity because we want to
>> allocate the space as part of the context header, but nothin too bad. We
>> might optimize it a bit by using a regular bitmap (instead of just an
>> array of bools), but I haven't done that.
>
>Hmm ... so if this is an array of bools, why isn't it declared bool*
>rather than char* ? (Pre-existing ugliness, sure, but we might as
>well fix it while we're here. Especially since you used sizeof(bool)
>in the space calculation.)
>

True. Will fix.

>I agree that maxaligning the start point of the array is pointless.
>
>I'd write "free chunks in a block" not "free chunks on a block",
>the latter seems rather shaky English. But that's getting picky.
>
>LGTM otherwise.
>

OK. Barring objections I'll push and backpatch this later today.

regards

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexey Kondratov 2020-01-16 17:09:09 Re: Physical replication slot advance is not persistent
Previous Message Tomas Vondra 2020-01-16 17:01:53 Re: SlabCheck leaks memory into TopMemoryContext