Re: pgsql: Introduce pg_shmem_allocations_numa view

From: Tomas Vondra <tomas(at)vondra(dot)me>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: Christoph Berg <myon(at)debian(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tomas(dot)vondra(at)postgresql(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Introduce pg_shmem_allocations_numa view
Date: 2025-06-24 09:20:15
Message-ID: a3a4fe3d-1a80-4e03-aa8e-150ee15f6c35@vondra.me
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 6/24/25 10:24, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Jun 24, 2025 at 03:43:19AM +0200, Tomas Vondra wrote:
>> On 6/23/25 23:47, Tomas Vondra wrote:
>>> ...
>>>
>>> Or maybe the 32-bit chroot on 64-bit host matters and confuses some
>>> calculation.
>>>
>>
>> I think it's likely something like this.
>
> I think the same.
>
>> I noticed that if I modify
>> pg_buffercache_numa_pages() to query the addresses one by one, it works.
>> And when I increase the number, it stops working somewhere between 16k
>> and 17k items.
>
> Yeah, same for me with pg_get_shmem_allocations_numa(). It works if
> pg_numa_query_pages() is done on chunks <= 16 pages but fails if done on more
> than 16 pages.
>
> It's also confirmed by test_chunk_size.c attached:
>
> $ gcc-11 -m32 -o test_chunk_size test_chunk_size.c
> $ ./test_chunk_size
> 1 pages: SUCCESS (0 errors)
> 2 pages: SUCCESS (0 errors)
> 3 pages: SUCCESS (0 errors)
> 4 pages: SUCCESS (0 errors)
> 5 pages: SUCCESS (0 errors)
> 6 pages: SUCCESS (0 errors)
> 7 pages: SUCCESS (0 errors)
> 8 pages: SUCCESS (0 errors)
> 9 pages: SUCCESS (0 errors)
> 10 pages: SUCCESS (0 errors)
> 11 pages: SUCCESS (0 errors)
> 12 pages: SUCCESS (0 errors)
> 13 pages: SUCCESS (0 errors)
> 14 pages: SUCCESS (0 errors)
> 15 pages: SUCCESS (0 errors)
> 16 pages: SUCCESS (0 errors)
> 17 pages: 1 errors
> Threshold: 17 pages
>
> No error if -m32 is not used.
>
>> It may be a coincidence, but I suspect it's related to the sizeof(void
>> *) being 8 in the kernel, but only 4 in the chroot. So the userspace
>> passes an array of 4-byte items, but kernel interprets that as 8-byte
>> items. That is, we call
>>
>> long move_pages(int pid, unsigned long count, void *pages[.count], const
>> int nodes[.count], int status[.count], int flags);
>>
>> Which (I assume) just passes the parameters to kernel. And it'll
>> interpret them per kernel pointer size.
>>
>
> I also suspect something in this area...
>
>> If this is what's happening, I'm not sure what to do about it ...
>
> We could work by chunks (16?) on 32 bits but would probably produce performance
> degradation (we mention it in the doc though). Also would always 16 be a correct
> chunk size?

I don't see how this would solve anything?

AFAICS the problem is the two places are confused about how large the
array elements are, and get to interpret that differently. Using a
smaller array won't solve that. The pg function would still allocate
array of 16 x 32-bit pointers, and the kernel would interpret this as 16
x 64-bit pointers. And that means the kernel will (a) write into memory
beyond the allocated buffer - a clear buffer overflow, and (b) see bogus
pointers, because it'll concatenate two 32-bit pointers.

I don't see how using smaller array makes this correct. That it works is
more a matter of luck, and also a consequence of still allocating the
whole array, so there's no overflow (at least I kept that, not sure how
you did the chunks).

If I fix the code to make the entries 64-bit (by treating the pointers
as int64), it suddenly starts working - no bad addresses, etc. Well,
almost, because I get this

bufferid | os_page_num | numa_node
----------+-------------+-----------
1 | 0 | 0
1 | 1 | -14
2 | 2 | 0
2 | 3 | -14
3 | 4 | 0
3 | 5 | -14
4 | 6 | 0
4 | 7 | -14
...

The -14 status is interesting, because that's the same value Christoph
reported as the other issue (in pg_shmem_allocations_numa).

I did an experiment and changed os_page_status to be declared as int64,
not just int. And interestingly, that produced this:

bufferid | os_page_num | numa_node
----------+-------------+-----------
1 | 0 | 0
1 | 1 | 0
2 | 2 | 0
2 | 3 | 0
3 | 4 | 0
3 | 5 | 0
4 | 6 | 0
4 | 7 | 0
...

But I don't see how this makes any sense, because "int" should be 4B in
both cases (in 64-bit kernel and 32-bit chroot).

FWIW I realized this applies to "official" systems with 32-bit user
space on 64-bit kernels, like e.g. rpi5 with RPi OS 32-bit. (Fun fact,
rpi5 has 8 NUMA nodes, with all CPUs attached to all NUMA nodes.)

I'm starting to think we need to disable NUMA for setups like this,
mixing 64-bit kernels with 32-bit chroot. Is there a good way to detect
those, so that we can error-out?

FWIW this doesn't explain the strange valgrind issue, though.

--
Tomas Vondra

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2025-06-24 09:40:48 pgsql: Fix virtual generated column type checking for ALTER TABLE
Previous Message Bertrand Drouvot 2025-06-24 08:24:53 Re: pgsql: Introduce pg_shmem_allocations_numa view

Browse pgsql-hackers by date

  From Date Subject
Next Message Nisha Moond 2025-06-24 09:37:28 Re: Logical Replication of sequences
Previous Message Bertrand Drouvot 2025-06-24 08:42:49 Re: POC: enable logical decoding when wal_level = 'replica' without a server restart