public inbox for [email protected]
help / color / mirror / Atom feedShmem allocated wrong for custom cumulative stats
5+ messages / 2 participants
[nested] [flat]
* Shmem allocated wrong for custom cumulative stats
@ 2026-04-06 00:16 Heikki Linnakangas <[email protected]>
2026-04-06 00:55 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Heikki Linnakangas @ 2026-04-06 00:16 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Michael Paquier <[email protected]>
There's only one call of ShmemAlloc() left in the tree outside shmem.c,
in StatsShmemInit(), and that call doesn't look right:
The StatsShmemSize() function takes those allocations into account when
it calculates the size for the "Shared Memory Stats" shmem area, but
ShmemAlloc() doesn't use that reservation, it uses the general-purpose
unreserved shmem that then shows up as "<anonymous>" in
pg_shmem_allocations. The space reserved with ShmemRequestStruct() /
ShmemInitStruct() goes unused.
We should use the memory that we've reserved, per the attached patch.
One consequence of this fix though is that the allocations are now only
MAXALIGNed, while ShmemAlloc() uses CACHELINEALIGN(). Not sure which we
want.
I noticed this while working on the new shmem allocation functions, but
it's a pre-existing bug in stable branches too.
- Heikki
Attachments:
[text/x-patch] 0001-Use-the-allocated-space-properly-for-custom-stats-ki.patch (1.1K, 2-0001-Use-the-allocated-space-properly-for-custom-stats-ki.patch)
download | inline diff:
From 62d9a2fc8a56751fcd8a8ab780db7781374db5f0 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Mon, 6 Apr 2026 02:54:18 +0300
Subject: [PATCH 1/1] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 955faf5ebc7..5e65a21fa83 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -189,6 +189,7 @@ StatsShmemInit(void *arg)
* efficiency win.
*/
ctl->raw_dsa_area = p;
+ p += pgstat_dsa_init_size();
dsa = dsa_create_in_place(ctl->raw_dsa_area,
pgstat_dsa_init_size(),
LWTRANCHE_PGSTATS_DSA, NULL);
@@ -242,7 +243,8 @@ StatsShmemInit(void *arg)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
--
2.47.3
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Shmem allocated wrong for custom cumulative stats
2026-04-06 00:16 Shmem allocated wrong for custom cumulative stats Heikki Linnakangas <[email protected]>
@ 2026-04-06 00:55 ` Michael Paquier <[email protected]>
2026-04-06 02:36 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2026-04-06 00:55 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Mon, Apr 06, 2026 at 03:16:47AM +0300, Heikki Linnakangas wrote:
> We should use the memory that we've reserved, per the attached patch. One
> consequence of this fix though is that the allocations are now only
> MAXALIGNed, while ShmemAlloc() uses CACHELINEALIGN(). Not sure which we
> want.
Indeed, it's not right. Thanks for the report. I am pretty sure that
I intended each chunk to be MAXALIGN()-d for each custom stats kind
registered, allocated in a non-anonymous way, without cache alignment.
> I noticed this while working on the new shmem allocation functions, but it's
> a pre-existing bug in stable branches too.
Right, down to v18 where this has been introduced. That's my bug, so
I'd be OK to take care of it myself, if you are OK with that of
course.
Actually, shouldn't StatsShmemSize() use an add_size() for each
shared_size? Noted while passing through the code, extra error from
the same commit.
--
Michael
From 9d0c00e96d8776d21f556e1457abf635fe211f9c Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v2] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 955faf5ebc7d..b8f354c818a0 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -150,8 +150,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -189,6 +188,7 @@ StatsShmemInit(void *arg)
* efficiency win.
*/
ctl->raw_dsa_area = p;
+ p += pgstat_dsa_init_size();
dsa = dsa_create_in_place(ctl->raw_dsa_area,
pgstat_dsa_init_size(),
LWTRANCHE_PGSTATS_DSA, NULL);
@@ -242,7 +242,8 @@ StatsShmemInit(void *arg)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
--
2.53.0
Attachments:
[text/plain] v2-0001-Use-the-allocated-space-properly-for-custom-stats.patch (1.3K, 2-v2-0001-Use-the-allocated-space-properly-for-custom-stats.patch)
download | inline diff:
From 9d0c00e96d8776d21f556e1457abf635fe211f9c Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v2] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 955faf5ebc7d..b8f354c818a0 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -150,8 +150,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -189,6 +188,7 @@ StatsShmemInit(void *arg)
* efficiency win.
*/
ctl->raw_dsa_area = p;
+ p += pgstat_dsa_init_size();
dsa = dsa_create_in_place(ctl->raw_dsa_area,
pgstat_dsa_init_size(),
LWTRANCHE_PGSTATS_DSA, NULL);
@@ -242,7 +242,8 @@ StatsShmemInit(void *arg)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
--
2.53.0
[application/pgp-signature] signature.asc (833B, 3-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Shmem allocated wrong for custom cumulative stats
2026-04-06 00:16 Shmem allocated wrong for custom cumulative stats Heikki Linnakangas <[email protected]>
2026-04-06 00:55 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
@ 2026-04-06 02:36 ` Michael Paquier <[email protected]>
2026-04-06 03:14 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2026-04-06 02:36 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Mon, Apr 06, 2026 at 09:55:14AM +0900, Michael Paquier wrote:
> Right, down to v18 where this has been introduced. That's my bug, so
> I'd be OK to take care of it myself, if you are OK with that of
> course.
Note: something is wrong with -m32, for both patches. Digging into
that..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Shmem allocated wrong for custom cumulative stats
2026-04-06 00:16 Shmem allocated wrong for custom cumulative stats Heikki Linnakangas <[email protected]>
2026-04-06 00:55 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
2026-04-06 02:36 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
@ 2026-04-06 03:14 ` Michael Paquier <[email protected]>
2026-04-07 03:04 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Michael Paquier @ 2026-04-06 03:14 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Mon, Apr 06, 2026 at 11:36:21AM +0900, Michael Paquier wrote:
> On Mon, Apr 06, 2026 at 09:55:14AM +0900, Michael Paquier wrote:
>> Right, down to v18 where this has been introduced. That's my bug, so
>> I'd be OK to take care of it myself, if you are OK with that of
>> course.
>
> Note: something is wrong with -m32, for both patches. Digging into
> that..
And I have been puzzled for a few minutes here, trying to figure out
if this was something in v18 or something with the new shmem routines.
It is nothing of the kind: test_custom_stats has been underestimating
its shared_size, using PgStat_StatCustomFixedEntry instead of
PgStatShared_CustomFixedEntry. Interesting copy-pasto, HEAD-only,
second bug.
The attached is working correctly. The v18 flavor is slightly
simpler.
--
Michael
From 943746c5b430b485e15c44a0b7bf099454cee7dd Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v3] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 7 ++++---
.../modules/test_custom_stats/test_custom_fixed_stats.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 955faf5ebc7d..b8f354c818a0 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -150,8 +150,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -189,6 +188,7 @@ StatsShmemInit(void *arg)
* efficiency win.
*/
ctl->raw_dsa_area = p;
+ p += pgstat_dsa_init_size();
dsa = dsa_create_in_place(ctl->raw_dsa_area,
pgstat_dsa_init_size(),
LWTRANCHE_PGSTATS_DSA, NULL);
@@ -242,7 +242,8 @@ StatsShmemInit(void *arg)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
diff --git a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
index f9e7c7172802..a066ce117a6d 100644
--- a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
@@ -50,7 +50,7 @@ static const PgStat_KindInfo custom_stats = {
.fixed_amount = true, /* exactly one entry */
.write_to_file = true, /* persist to stats file */
- .shared_size = sizeof(PgStat_StatCustomFixedEntry),
+ .shared_size = sizeof(PgStatShared_CustomFixedEntry),
.shared_data_off = offsetof(PgStatShared_CustomFixedEntry, stats),
.shared_data_len = sizeof(((PgStatShared_CustomFixedEntry *) 0)->stats),
--
2.53.0
From f6787781026c7dcf6c9a38b0e7f8c8427a7b2da6 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v3] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 08ec264baa3a..2b7f783ef7c4 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -142,8 +142,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -227,7 +226,8 @@ StatsShmemInit(void)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
--
2.53.0
Attachments:
[text/plain] v3-0001-Use-the-allocated-space-properly-for-custom--HEAD.patch (2.1K, 2-v3-0001-Use-the-allocated-space-properly-for-custom--HEAD.patch)
download | inline diff:
From 943746c5b430b485e15c44a0b7bf099454cee7dd Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v3] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 7 ++++---
.../modules/test_custom_stats/test_custom_fixed_stats.c | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 955faf5ebc7d..b8f354c818a0 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -150,8 +150,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -189,6 +188,7 @@ StatsShmemInit(void *arg)
* efficiency win.
*/
ctl->raw_dsa_area = p;
+ p += pgstat_dsa_init_size();
dsa = dsa_create_in_place(ctl->raw_dsa_area,
pgstat_dsa_init_size(),
LWTRANCHE_PGSTATS_DSA, NULL);
@@ -242,7 +242,8 @@ StatsShmemInit(void *arg)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
diff --git a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
index f9e7c7172802..a066ce117a6d 100644
--- a/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_fixed_stats.c
@@ -50,7 +50,7 @@ static const PgStat_KindInfo custom_stats = {
.fixed_amount = true, /* exactly one entry */
.write_to_file = true, /* persist to stats file */
- .shared_size = sizeof(PgStat_StatCustomFixedEntry),
+ .shared_size = sizeof(PgStatShared_CustomFixedEntry),
.shared_data_off = offsetof(PgStatShared_CustomFixedEntry, stats),
.shared_data_len = sizeof(((PgStatShared_CustomFixedEntry *) 0)->stats),
--
2.53.0
[text/plain] v3-0001-Use-the-allocated-space-properly-for-custom-s-v18.patch (1.1K, 3-v3-0001-Use-the-allocated-space-properly-for-custom-s-v18.patch)
download | inline diff:
From f6787781026c7dcf6c9a38b0e7f8c8427a7b2da6 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2026 09:47:02 +0900
Subject: [PATCH v3] Use the allocated space properly for custom stats kind
---
src/backend/utils/activity/pgstat_shmem.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index 08ec264baa3a..2b7f783ef7c4 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -142,8 +142,7 @@ StatsShmemSize(void)
continue;
Assert(kind_info->shared_size != 0);
-
- sz += MAXALIGN(kind_info->shared_size);
+ sz = add_size(sz, MAXALIGN(kind_info->shared_size));
}
return sz;
@@ -227,7 +226,8 @@ StatsShmemInit(void)
int idx = kind - PGSTAT_KIND_CUSTOM_MIN;
Assert(kind_info->shared_size != 0);
- ctl->custom_data[idx] = ShmemAlloc(kind_info->shared_size);
+ ctl->custom_data[idx] = p;
+ p += MAXALIGN(kind_info->shared_size);
ptr = ctl->custom_data[idx];
}
--
2.53.0
[application/pgp-signature] signature.asc (833B, 4-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Shmem allocated wrong for custom cumulative stats
2026-04-06 00:16 Shmem allocated wrong for custom cumulative stats Heikki Linnakangas <[email protected]>
2026-04-06 00:55 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
2026-04-06 02:36 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
2026-04-06 03:14 ` Re: Shmem allocated wrong for custom cumulative stats Michael Paquier <[email protected]>
@ 2026-04-07 03:04 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Michael Paquier @ 2026-04-07 03:04 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Mon, Apr 06, 2026 at 12:14:06PM +0900, Michael Paquier wrote:
> The attached is working correctly. The v18 flavor is slightly
> simpler.
I have looked at it again today, and noticed that the test modules of
both HEAD and REL_18_STABLE were incorrect in the values they set for
shared_size for the fixed-sized cases. In both cases we were
finishing with an underestimation. I could only see a direct impact
on HEAD for the 32-bit builds, though, but the problem was the same.
The allocation has been fixed in 17132f55c5a2, with the modules
handled by 98979578055f.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2026-04-07 03:04 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-06 00:16 Shmem allocated wrong for custom cumulative stats Heikki Linnakangas <[email protected]>
2026-04-06 00:55 ` Michael Paquier <[email protected]>
2026-04-06 02:36 ` Michael Paquier <[email protected]>
2026-04-06 03:14 ` Michael Paquier <[email protected]>
2026-04-07 03:04 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox