At Fri, 20 May 2022 12:00:14 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> At Thu, 19 May 2022 17:16:03 -0400, Tom Lane <t...@sss.pgh.pa.us> wrote in
> > Justin Pryzby <pry...@telsasoft.com> writes:
> > > ./tmp_install/usr/local/pgsql/bin/postgres -D
> > > ./src/test/regress/tmp_check/data -c min_dynamic_shared_memory=1MB
> > > TRAP: FailedAssertion("val > base", File:
> > > "../../../../src/include/utils/relptr.h", Line: 67, PID: 21912)
> >
> > Yeah, I see it too.
> >
> > > It looks like this may be pre-existing problem exposed by
> > > commit e07d4ddc55fdcf82082950b3eb0cd8f728284c9d
> >
> > Agreed. Here I see
> >
> > #5 FreePageManagerInitialize (fpm=fpm@entry=0x7f34b3ddd300,
> > base=base@entry=0x7f34b3ddd300 "") at freepage.c:187
> > #6 0x000000000082423e in dsm_shmem_init () at dsm.c:473
> >
> > so that where we do
> >
> > relptr_store(base, fpm->self, fpm);
> >
> > the "relative" pointer value would have to be zero, making the case
> > indistinguishable from a NULL pointer. We can either change the
> > caller so that these addresses aren't the same, or give up the
> > ability to store NULL in relptrs ... doesn't seem like a hard call.
> >
> > One interesting question I didn't look into is why it takes a nondefault
> > value of min_dynamic_shared_memory to expose this bug.
>
> The path is taken only when a valid value is given to the
> parameter. If we don't use preallocated dsm, it is initialized
> elsewhere. In those cases the first bytes of the base address (the
> second parameter of FreePageManagerInitialize) are used for
> dsa_segment_header so the relptr won't be zero (!= NULL).
>
> It can be silenced by wasting the first MAXALIGN bytes of
> dsm_main_space_begin..
Actually, that change doesn't result in wasting of usable memory size
since the change doesn't move the first effective page.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 9d86bbe872..c67134ec27 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -461,12 +461,19 @@ dsm_shmem_init(void)
dsm_main_space_begin = ShmemInitStruct("Preallocated DSM", size, &found);
if (!found)
{
- FreePageManager *fpm = (FreePageManager *) dsm_main_space_begin;
+ /*
+ * fpm cannot be the same with dsm_main_space_begin due to the
+ * restriction imposed by FreePageManagerInitialize() due to
+ * relptr. Add MAXALIGN(1) to fpm to live with that restriction.
+ */
+ int gap = MAXALIGN(1);
+ FreePageManager *fpm =
+ (FreePageManager *) ((char *)dsm_main_space_begin + gap);
size_t first_page = 0;
size_t pages;
/* Reserve space for the FreePageManager. */
- while (first_page * FPM_PAGE_SIZE < sizeof(FreePageManager))
+ while (first_page * FPM_PAGE_SIZE < sizeof(FreePageManager) + gap)
++first_page;
/* Initialize it and give it all the rest of the space. */
diff --git a/src/test/modules/test_misc/t/004_check_min_dsm_size.pl b/src/test/modules/test_misc/t/004_check_min_dsm_size.pl
new file mode 100644
index 0000000000..025dca33e4
--- /dev/null
+++ b/src/test/modules/test_misc/t/004_check_min_dsm_size.pl
@@ -0,0 +1,14 @@
+# Tests of min_dynamic_shared_memory
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf', 'min_dynamic_shared_memory = 1MB');
+$node->start;
+pass('check if server can start with min_dynamic_shared_memory');
+done_testing();
