public inbox for [email protected]help / color / mirror / Atom feed
Shmem 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]> 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:55 Michael Paquier <[email protected]> parent: Heikki Linnakangas <[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 02:36 Michael Paquier <[email protected]> parent: 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 03:14 Michael Paquier <[email protected]> parent: 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-07 03:04 Michael Paquier <[email protected]> parent: 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