Re: Better shared data structure management and resizable shared data structures

Hi,

The CF bot has been flagging the patch set as "Needs rebase" for the last day
or so. The failure is not caused by any change on master -- v20260810-0001
still applies cleanly on 1d1d7b0e9c9. The conflict is inside the series:
v20260810-0003 (the smaps-walker diagnostic elog patch) was generated with
0001 as its parent, so it overlaps the same block in test_shmem.c that
v20260810-0002 rewrites (the shared/private VMA check and the switch from
int64 to long counters). Applied in order 0001 -> 0002 -> 0003, hunk #2 of
0003 rejects.

0003 was only meant as a temporary diagnostic to inspect /proc/self/smaps
output while investigating the earlier 32-bit failure, and is not intended
for commit. Dropping it removes the conflict.

Attached is v20260811, just the two commits that are actually proposed for
commit:

v20260811-0001-Resizable-shared-memory-structures.patch
v20260811-0002-test_shmem-make-resizable-shmem-tests-portable.patch

Verified with `git am` on postgres/master @ 1d1d7b0e9c9; both apply cleanly
and the test_shmem TAP suite (001_fixed_shmem_struct,
002_resizable_shmem_struct)
passes locally.

No code changes vs v20260810-0001 / v20260810-0002 -- the diffs are byte
identical, only the diagnostic 0003 is dropped and the series is renumbered.

Thanks,
Palak

On Mon, 10 Aug 2026 at 23:06, Palak Chaturvedi
<chaturvedipalak1911(at)gmail(dot)com> wrote:
>
> Hi Ashutosh,
> Attached is a test-portability fix on top of v20260724-0006 to unblock
> CFbot [1], plus the small diagnostic patch I used to root-cause it, in
> case it's useful later.
>
> v20260810-0001 = your v20260724-0006, unchanged, renumbered so
> CFbot picks up the set as one series
> v20260810-0002 = the proposed fix (test module only; no changes
> outside src/test/modules/test_shmem/)
> v20260810-0003 = diagnostic elog around the existing smaps walker
> (please don't commit this one; it's here so the
> same instrumentation can be reused later)
>
> Also attached:
> smaps-trace-64bit.txt full 64-bit trace, 12 pids
> smaps-trace-32bit.txt per-pid header + first 8 include lines +
> elided marker + stop/result; the full trace
> is available by rerunning 0003
>
> Method
> ------
>
> To root-cause this rather than guess from the CFbot failure messages,
> I forked off the CF tip (50911a16240 with v20260724-0006 applied),
> added logging only to test_shmem_usage() (0003), pushed to trigger a
> GitHub Actions run [2], and read the archived server logs. The green
> run of 0001+0002 is at [3] (9 jobs pass: Linux 32, Linux 64, macOS,
> Windows MinGW, both Windows VS slices, plus SanityCheck,
> CompilerWarnings, Linux Autoconf).
>
> Diff summary of 0002 vs your patch:
>
> src/test/modules/test_shmem/t/002_resizable_shmem_struct.pl | 13 +++++-
> src/test/modules/test_shmem/test_shmem.c | 50
> ++++++++--------
> 2 files changed, 35 insertions(+), 28 deletions(-)
>
> Findings
> --------
>
> 1. test_shmem_usage() smaps walker crosses the mapping boundary
> ---------------------------------------------------------------
>
> The walker only checks address contiguity (start == prev_end) between
> adjacent VMAs, so the first VMA of any kind that happens to abut the
> target /dev/zero (deleted) segment is silently pulled into the RSS
> sum. The comment on the loop mentions mprotect() splitting the
> mapping, but the check itself is unaware of the split's identity.
>
> On Linux 64-bit CI, for the "shrink to minimum, session 1" scenario
> (one of the smallest overages, +139264 bytes over sum(allocated_size)):
>
> include target VMA: 7f1b84b0c000-7f1b84de6000 rw-s ... 10315
> /dev/zero (deleted) Rss=1116 kB, total=1116 kB
> include contiguous VMA: 7f1b84de6000-7f1b9dde5000 ---s ... 10315
> /dev/zero (deleted) Rss=0 kB, total=1116 kB
> include contiguous VMA: 7f1b9dde5000-7f1b9de00000 rw-s ... 10315
> /dev/zero (deleted) Rss=0 kB, total=1116 kB
> include contiguous VMA: 7f1b9de00000-7f1b9e600000 rw-p 00000000 00:00 0
> Rss=2048 kB, total=3164 kB <-- adjacent private anonymous VMA
> stop before VMA: 7f1b9e6f7000-7f1b9f200000 rw-p 00000000 00:00 0
> result: rss=3164 kB swap=0 kB shared_hugetlb=0 kB bytes=3239936
>
> Reported shmem_usage is 3164 kB; real shared-segment RSS is 1116 kB.
> The 2048 kB delta is one adjacent private anonymous VMA that happens
> to sit right after the shared segment. Across the 12 failing
> accounting scenarios on 64-bit, the same 2048 kB VMA at
> 7f1b9de00000-7f1b9e600000 is wrongly included every time, and the
> walker's final "bytes=" matches the TAP-reported shmem_usage exactly
> (full log in smaps-trace-64bit.txt).
>
> On 32-bit the same bug is much worse. The walker crosses out of the
> target /dev/zero segment, through one anonymous VMA, and then straight
> into the writable segments of loaded libraries: libcap-ng, libresolv,
> libc, libssl, libcrypto, libm, libicu*, libkrb5*, libldap, liburing,
> libxml2, libzstd, libstdc++, libsystemd, libubsan, and several more,
> before it finally stops at another SysV shared segment. Per-pid,
> roughly 170 adjacent VMAs are wrongly included; overage is about 2 MB
> per scenario. Header excerpt for one pid (target=0xd35d9700), full
> per-pid summary in smaps-trace-32bit.txt:
>
> include target VMA: d3325000-d99da000 rw-s 00000000 00:01 8414
> /dev/zero (deleted)
> include contiguous VMA: d99da000-ec5d9000 ---s 066b5000 00:01 8414
> /dev/zero (deleted)
> include contiguous VMA: ec5d9000-ec5f3000 rw-s 192b4000 00:01 8414
> /dev/zero (deleted)
> include contiguous VMA: ec5f3000-ec5f9000 rw-p 00000000 00:00 0
> include contiguous VMA: ec5f9000-ec5fa000 r--p ...
> /usr/lib/i386-linux-gnu/libcap-ng.so.0.0.0
> ...
> ... (168 more contiguous VMAs elided) ...
> stop before VMA: f02dc000-f02dd000 rw-s ... /SYSV0088a83c (deleted)
> result: rss=<garbage, see Finding #2> bytes=109752320
>
> What 0002 proposes (test_shmem.c): also check permissions[3] (the 's'
> vs 'p' character) when continuing across a split. This still handles
> the case the loop comment cares about, one MAP_SHARED region split by
> mprotect() into a chain of 'rw-s' / '---s' / 'rw-s' adjacent VMAs
> backed by the same inode, while excluding adjacent private mappings.
>
> One thing I'd like your opinion on: the 's' check by itself is not
> airtight. If a *different* MAP_SHARED mapping (different inode /
> pathname) happened to sit immediately contiguous to our target
> segment, the walker would still include it. I haven't hit this on the
> CI runners we tested, but on more crowded address spaces (a backend
> that has attached to additional SysV segments, for example) it seems
> plausible. One option would be to also match on the inode+pathname of
> the target VMA (parse them from the same sscanf) and stop as soon as
> they change. That's a bit more parsing, not much. Happy to add it to
> 0002 or leave it as a follow-up, whichever you'd prefer.
>
> 2. sscanf("%ld", int64 *) is a width mismatch on MSVC and 32-bit Linux
> ----------------------------------------------------------------------
>
> The CF-base test_shmem_usage() has:
>
> int64 val;
> ...
> if (sscanf(line, "Rss: %ld kB", &val) == 1)
> total_rss_kb += val;
>
> sscanf("%ld", ...) writes a sizeof(long) value, but val is int64. On
> platforms where long is 32 bits (MSVC on any target, 32-bit Linux),
> sscanf writes only the low 32 bits and leaves the upper 32 bits of
> val uninitialized. This is what makes the totals in the 32-bit trace
> look like garbage: 0003 uses INT64_FORMAT and prints val correctly,
> so what you see is the CF-base sscanf's half-populated int64.
>
> The returned bytes= is still correct because total_rss_kb passes
> through mul_size()/add_size() (unsigned Size, which is 32 bits on
> 32-bit Linux) and the upper garbage gets truncated. So the accounting
> failure isn't hidden -- the delta that TAP compares is right, only
> the diagnostic printout looks wrong.
>
> 0002 switches the four accounting variables (total_rss_kb,
> total_swap_kb, total_shared_hugetlb_kb, val) from int64 to plain
> 'long', which matches %ld on every supported platform and is more
> than wide enough for kB-per-VMA counts.
>
> 3. read-past-current-size test: compiler elides the read loop
> -------------------------------------------------------------
>
> The read side of resizable_shmem_access_beyond_size() had:
>
> for (int i = resizable_shmem->num_entries; i < test_max_entries; i++)
> sink = resizable_shmem->data[i];
>
> At -O2 the compiler can legally elide most of that loop: 'sink' is a
> plain static int32, resizable_shmem->data is a non-volatile pointer,
> and only the last store to sink is externally observable. The CF-base
> comment ("Return the last read value so that compiler doesn't optimize
> away the assignment to sink") predicts the risk but doesn't prevent
> it -- returning sink from the SQL function only pins the last
> iteration.
>
> When the read is elided, the backend never faults, so tests 45-47
> fail like this (base + 0003 diagnostic run on Linux 64-bit):
>
> not ok 45 - read past current size crashes the backend
> not ok 46 - read crash reports lost connection
> got: '' (empty stderr -- backend didn't crash)
> not ok 47 - read succeeds after crash recovery
> got: 'f' expected: 't' (no crash, so no recovery)
>
> 0002 fixes this by casting through a volatile pointer:
>
> volatile int32 *volatile_data = resizable_shmem->data;
> for (int i = resizable_shmem->num_entries; i < test_max_entries; i++)
> sink = volatile_data[i];
>
> Each volatile_data[i] load is now a required side-effect and can't be
> elided, so the read actually faults.
>
> 4. Windows: skip 002_resizable_shmem_struct.pl
> ----------------------------------------------
>
> Resizable shared memory isn't supported on Windows yet, and the test
> relies on /proc/self/smaps, which is Linux-only. 0002 adds an early
> `plan skip_all => 'resizable shared memory tests are not supported on
> Windows'` so the two Windows CFbot jobs stop failing. If a future
> revision brings Windows support, we can drop the skip.
>
> [1] https://commitfest.postgresql.org/patch/6652/
> [2] Diagnostic run (CF base + 0003 elog only, expected failure):
> https://github.com/palak-chaturvedi/postgres/actions/runs/31178059423
> [3] All-green run of 0001+0002 across 9 jobs:
> https://github.com/palak-chaturvedi/postgres/actions/runs/31190837956
>
> Thanks,
> Palak