Re: Reducing the chunk header sizes on all memory context types

On 8/30/22 03:16, David Rowley wrote:
> On Tue, 30 Aug 2022 at 12:45, David Rowley <dgrowleyml@gmail.com> wrote:
>> I think the existing sentinel check looks wrong:
>>
>> if (!sentinel_ok(chunk, slab->chunkSize))
>>
>> shouldn't that be passing the pointer rather than the chunk?
> 
> Here's v2 of the slab-fix patch.
> 
> I've included the sentinel check fix.  This passes make check-world
> for me when do a 32-bit build on my x86_64 machine and adjust
> pg_config.h to set MAXIMUM_ALIGNOF to 8.
> 
> Any chance you could run make check-world on your 32-bit Raspberry PI?
> 

Will do, but I think the sentinel fix should be

   if (!sentinel_ok(chunk, Slab_CHUNKHDRSZ + slab->chunkSize))

which is what the other contexts do. However, considering check-world
passed even before the sentinel_ok fix, I'm a bit skeptical about that
proving anything.

FWIW I added a WARNING to SlabCheck before the condition guarding the
sentinel check, printing the (full) chunk size and header size, and this
is what I got in test_decoding (deduplicated):

armv7l (32-bit rpi4)

+WARNING:  chunkSize 216 fullChunkSize 232 header 16
+WARNING:  chunkSize 64 fullChunkSize 80 header 16

aarch64 (64-bit rpi4)

+WARNING:  chunkSize 304 fullChunkSize 320 header 16
+WARNING:  chunkSize 80 fullChunkSize 96 header 16

So indeed, those are *perfect* matches and thus the sentinel_ok() never
executed. So no failures until now. On x86-64 I get the same thing as on
aarch64. I guess that explains why it never failed. Seems like a pretty
amazing coincidence ...


> I'm also wondering if this should also be backpatched back to v10,
> providing the build farm likes it well enough on master.

I'd say the sentinel fix may need to be backpatched.


regard

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company