Re: Changing shared_buffers without restart

From: Palak Chaturvedi <chaturvedipalak1911(at)gmail(dot)com>
To: Yuhang Qiu <iamqyh(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Heikki Linnakangas <heikki(dot)linnakangas(at)databricks(dot)com>, Haoyu Huang <haoyu(dot)huang(at)databricks(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Jakub Wartak <jakub(dot)wartak(at)enterprisedb(dot)com>
Subject: Re: Changing shared_buffers without restart
Date: 2026-09-07 13:00:55
Message-ID: CALfch1_K04G39nNrZCBw+pbqG4aXMXsAcv04A_MRhEtjJi2T8A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Yuhang,

On Thu, 27 Aug 2026 at 14:31, Yuhang Qiu <iamqyh(at)gmail(dot)com> wrote:
>
> Hi Palak,
>
> 0010 and 0011 both look good to me.
>
Thanks
> I missed the existing `HAVE_RESIZABLE_SHMEM` guard. Ignore that comment.
>
Got it.
> > It seems whether this actually PANICs depends on the kernel. On my box
> > (Linux 6.17, huge_pages=off) those over-wide MADV_REMOVE calls all
> > return 0, so I don't hit the PANIC locally.
>
> I reproduced it on Linux 5.10 with `huge_pages=off`. The code calls
> `MADV_REMOVE` from the new end to `maximum_size`, crossing from the RW
> area into the existing `PROT_NONE` tail. The call returns EACCES and the
> resize PANICs.
>
> I tested this fix:
> ```c
> char *current_end = (char *) TYPEALIGN(page_size,
> (char *) result->location + result->size);
> char *reserved_end = (char *) TYPEALIGN_DOWN(page_size,
> (char *) result->location + result->maximum_size);
> char *max_end = Min(current_end, reserved_end);
> ```
>

Tried this. This works fine. Attached a patch with this. Also did the testing
on my box with huge_pages=off and confirmed the EACCES/PANIC before the
fix and a clean resize after.

> The first bound stops at the current allocation; the second preserves the
> last page when it is shared with the next structure. Linux 6.7 changed
> the check from `VM_WRITE` to `VM_MAYWRITE` [1], which explains why the
> over-wide call succeeds on 6.17.
>
> > I did try to reproduce it, including widening the window artificially,
> > but couldn't catch it. It seems to need AIO in the mix.
>
> The race is:
> 1. A backend takes a buffer above the shrink target before processing the
> new-allocation barrier.
> 2. It publishes the mapping entry (`BM_TAG_VALID` is set), while `BM_VALID`
> is still clear.
> 3. It processes the barrier before the read completes. This can also
> happen on the synchronous path between `StartReadBuffers()` and
> `WaitReadBuffers()`.
> 4. `EvictExtraBuffers()` sees `BM_VALID` clear and skips the buffer, leaving
> the mapping entry behind after the shrink.
>
> To force it, hold the read completion after step 2, let the backend process
> the barrier, and then start the shrink.
>
> Both the precheck and the locked assertion should use `BM_TAG_VALID`, and
> the latter should check the state returned by `LockBufHdr()`. The shrink
> will then roll back instead of leaving an orphaned entry.
>

Thanks for the steps. Reproduced this using the steps via creating a test
file and adding an injection point right after BM_TAG_VALID is set, before
the read completes. Was able to fail it at the Assert (signal 6) with the
code as posted. Not attaching the test file or injection point as part of
the patch, happy to share the test
file separately if useful, just let me know.

Here is the patch for EvictExtraBuffers. Changed both the precheck and the
locked assertion to BM_TAG_VALID, and fixed the assertion to check the
state returned by LockBufHdr() instead of the stale unlocked read. With
this, the shrink rolls back and logs the pinned buffer instead of leaving
an orphaned mapping. Also went through every other function in bufmgr.c
that walks buffers and checks BM_VALID (EvictAllUnpinnedBuffers,
EvictRelUnpinnedBuffers, SyncOneBuffer, FlushRelationBuffers,
FlushDatabaseBuffers, the MarkDirty helpers) — none of those are affected,
a mid-IO buffer can't be dirty so skipping it there is correct.

> Running `autoconf` will update `configure` with new `configure.ac`.
>

Right, configure.ac has AC_CHECK_DECLS for MADV_POPULATE_WRITE and
MADV_REMOVE but configure was never regenerated, so autotools builds
never get HAVE_RESIZABLE_SHMEM defined. Ran autoconf 2.69 and attached
the regenerated configure.

> > Could you share the error you saw, or how to reproduce it?
> I tested it with `huge_pages = off` on Linux 5.10. The error is EACCES which
> I explained above.
>
> Best regards,
> Yuhang Qiu
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e8e17ee90eaf650c855adb0a3e5e965fd6692ff1
>

Thanks,
Palak

Attachment Content-Type Size
v20260907-0013-buffermgr-fix-EvictExtraBuffers-BM_TAG_VALID.patch application/octet-stream 1.3 KB
v20260907-0012-shmem-fix-madv-remove-range.patch application/octet-stream 979 bytes
v20260907-0014-configure-regenerate-for-MADV-checks.patch application/octet-stream 1.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2026-09-07 13:14:39 Re: Revert RI fast-path batching from REL_19_STABLE
Previous Message vignesh C 2026-09-07 12:55:35 Publication DDL can race with a concurrent UPDATE