Re: pgsql: Introduce pg_shmem_allocations_numa view

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

To: Tomas Vondra <tomas@vondra.me>

Cc: Christoph Berg <myon@debian.org>, Andres Freund <andres@anarazel.de>, Tomas Vondra <tomas.vondra@postgresql.org>, pgsql-hackers@lists.postgresql.org

Date: 2025-06-24T11:10:24Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Handle EPERM in pg_numa_init
- 599336c64fc9 19 (unreleased) landed
- 482e98ac4302 18.2 landed
Add CHECK_FOR_INTERRUPTS into pg_numa_query_pages
- 54ac4944c36f 18.0 landed
- bf1119d74a79 19 (unreleased) landed
Silence valgrind about pg_numa_touch_mem_if_required
- 14e52227e578 18.0 landed
- 81f287dc923f 19 (unreleased) landed
Limit the size of numa_move_pages requests
- 45879f48f140 18.0 landed
- 7fe2f67c7c9f 19 (unreleased) landed
Introduce pg_shmem_allocations_numa view
- 8cc139bec34a 18.0 cited

Hi,

On Tue, Jun 24, 2025 at 11:20:15AM +0200, Tomas Vondra wrote:
> On 6/24/25 10:24, Bertrand Drouvot wrote:
> > 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.
> > 
> > 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.

> I don't see how using smaller array makes this correct. That it works is
> more a matter of luck,

Not sure it's luck, maybe the wrong pointers arithmetic has no effect if batch
size is <= 16.

So we have kernel_move_pages() -> kernel_move_pages() (because nodes is NULL here
for us as we call "numa_move_pages(pid, count, pages, NULL, status, 0);").

So, if we look at do_pages_stat() ([1]), we can see that it uses an hardcoded
"#define DO_PAGES_STAT_CHUNK_NR 16UL" and that this pointers arithmetic:

"
        pages += chunk_nr;
        status += chunk_nr;
"

is done but has no effect since nr_pages will exit the loop if we use a batch
size <= 16.

So if this pointer arithmetic is not correct, (it seems that it should advance
by 16 * sizeof(compat_uptr_t) instead) then it has no effect as long as the batch
size is <= 16.

Does test_chunk_size also fails at 17 for you?

[1]: https://github.com/torvalds/linux/blob/master/mm/migrate.c

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com