pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedUpdate outdated references to SLRU ControlLock
8+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2025-09-05 567d27e8e2 a730ad8551 fd3b972d78 Fix outdated comments in slru.c
2025-09-03 c6ea528b47 399f7ce3da 24ee8678f3 Update outdated references to the SLRU ControlLock
* Update outdated references to SLRU ControlLock
@ 2025-09-01 03:32 Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 8+ messages in thread
From: Julien Rouhaud @ 2025-09-01 03:32 UTC (permalink / raw)
To: pgsql-hackers@lists.postgresql.org
Hi,
I just noticed that some references so the now removed SLRU ControlLock were
still present in slru.[ch]. I tried to improve the situation with the attached
patch by using the new "bank control lock" name that is used nearby.
Note that the main comment of slru.c still has one paragraph that mentions
"bank control lock" consistently before switching to just "control lock" in the
next paragraph. I'm assuming that it's ok in that context as it seems clear to
me that those are the same thing, just spelled with a less verbose name.
Attachments:
0001-Update-outdated-references-to-SLRU-ControlLock.patch (text/plain, 2.1K) download
From 8171ed0a87d998bd07dad6700bee0938b9fa49d9 Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Mon, 1 Sep 2025 11:16:01 +0800
Subject: [PATCH] Update outdated references to SLRU ControlLock
Oversight in 53c2a97a9266 which removed the single ControlLock and added the
bank control locks.
---
src/backend/access/transam/slru.c | 8 ++++----
src/include/access/slru.h | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 10ec259f382..f78e35d66d5 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -401,9 +401,9 @@ SimpleLruZeroPage(SlruCtl ctl, int64 pageno)
* Assume this page is now the latest active page.
*
* Note that because both this routine and SlruSelectLRUPage run with
- * ControlLock held, it is not possible for this to be zeroing a page that
- * SlruSelectLRUPage is going to evict simultaneously. Therefore, there's
- * no memory barrier here.
+ * bank control lock held, it is not possible for this to be zeroing a
+ * page that SlruSelectLRUPage is going to evict simultaneously.
+ * Therefore, there's no memory barrier here.
*/
pg_atomic_write_u64(&shared->latest_page_number, pageno);
@@ -437,7 +437,7 @@ SimpleLruZeroLSNs(SlruCtl ctl, int slotno)
* This is a convenience wrapper for the common case of zeroing a page and
* immediately flushing it to disk.
*
- * Control lock is acquired and released here.
+ * Bank control lock is acquired and released here.
*/
void
SimpleLruZeroAndWritePage(SlruCtl ctl, int64 pageno)
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 20dbd1e0070..0b81bc62639 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -55,7 +55,7 @@ typedef enum
/*
* Shared-memory state
*
- * ControlLock is used to protect access to the other fields, except
+ * Bank control locks are used to protect access to the other fields, except
* latest_page_number, which uses atomics; see comment in slru.c.
*/
typedef struct SlruSharedData
--
2.51.0
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
@ 2025-09-01 05:05 ` Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2025-09-01 05:05 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Mon, Sep 01, 2025 at 11:32:41AM +0800, Julien Rouhaud wrote:
> Note that the main comment of slru.c still has one paragraph that mentions
> "bank control lock" consistently before switching to just "control lock" in the
> next paragraph. I'm assuming that it's ok in that context as it seems clear to
> me that those are the same thing, just spelled with a less verbose name.
Good catch, right.
I am not seeing "control" used much as a term (3 times on HEAD).
There is a lot of "bank lock" or "SLRU bank lock", both being mixed
depending on the parts of the code using SLRUs (multixact, predicates,
etc.).
"SLRU bank lock" speaks a bit better to me, as the concept relates
to.. SLRUs, but that's mostly a matter of taste between the three
wordings, I guess. Do you have a preference?
Not saying that one is better than the other as they're all used, just
noticed the difference in the comments, with all the terms referring
to the same thing.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
@ 2025-09-01 05:19 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 8+ messages in thread
From: Julien Rouhaud @ 2025-09-01 05:19 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org
On Mon, Sep 01, 2025 at 02:05:56PM +0900, Michael Paquier wrote:
> On Mon, Sep 01, 2025 at 11:32:41AM +0800, Julien Rouhaud wrote:
> > Note that the main comment of slru.c still has one paragraph that mentions
> > "bank control lock" consistently before switching to just "control lock" in the
> > next paragraph. I'm assuming that it's ok in that context as it seems clear to
> > me that those are the same thing, just spelled with a less verbose name.
>
> Good catch, right.
>
> I am not seeing "control" used much as a term (3 times on HEAD).
> There is a lot of "bank lock" or "SLRU bank lock", both being mixed
> depending on the parts of the code using SLRUs (multixact, predicates,
> etc.).
Yes, and some other parts simply mentions "lock" (eg TransactionIdGetStatus)
> "SLRU bank lock" speaks a bit better to me, as the concept relates
> to.. SLRUs, but that's mostly a matter of taste between the three
> wordings, I guess. Do you have a preference?
I don't really have a preference. Bank lock is shorter but may be a bit more
obscure, especially outside slru.c, so using "SLRU bank lock" could be better
indeed.
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
@ 2025-09-03 01:21 ` Michael Paquier <michael@paquier.xyz>
2025-09-03 02:22 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2025-09-03 01:21 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Mon, Sep 01, 2025 at 01:19:41PM +0800, Julien Rouhaud wrote:
> I don't really have a preference. Bank lock is shorter but may be a bit more
> obscure, especially outside slru.c, so using "SLRU bank lock" could be better
> indeed.
Okay, I have used "SLRU bank lock" and backpatched that down to v17
where it matters.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
@ 2025-09-03 02:22 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 01:47 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Julien Rouhaud @ 2025-09-03 02:22 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org
On Wed, Sep 03, 2025 at 10:21:54AM +0900, Michael Paquier wrote:
> On Mon, Sep 01, 2025 at 01:19:41PM +0800, Julien Rouhaud wrote:
> > I don't really have a preference. Bank lock is shorter but may be a bit more
> > obscure, especially outside slru.c, so using "SLRU bank lock" could be better
> > indeed.
>
> Okay, I have used "SLRU bank lock" and backpatched that down to v17
> where it matters.
Thanks a lot!
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-03 02:22 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
@ 2025-09-05 01:47 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 05:11 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 8+ messages in thread
From: Julien Rouhaud @ 2025-09-05 01:47 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org
Hi,
On Wed, Sep 03, 2025 at 10:22:37AM +0800, Julien Rouhaud wrote:
> On Wed, Sep 03, 2025 at 10:21:54AM +0900, Michael Paquier wrote:
> > Okay, I have used "SLRU bank lock" and backpatched that down to v17
> > where it matters.
I caught a couple of other minor outdated things in the comments when finishing
rebasing my own work:
- long_segment_names has been added to SimpleLruInit
- SlruRecentlyUsed is not a macro anymore
Simple patch attached.
Attachments:
0001-Fix-a-few-outdated-SLRU-comments.patch (text/plain, 1.4K) download
From 392e74d26fdab34d1c3ed87cca58f06c92754d0b Mon Sep 17 00:00:00 2001
From: Julien Rouhaud <julien.rouhaud@free.fr>
Date: Fri, 5 Sep 2025 09:04:58 +0800
Subject: [PATCH] Fix a few outdated SLRU comments
long_segment_names has been added to SimpleLruInit, and SlruRecentlyUsed is now
a (static inline) function.
---
src/backend/access/transam/slru.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index f78e35d66d5..7ef52a6da0f 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -246,6 +246,7 @@ SimpleLruAutotuneBuffers(int divisor, int max)
* buffer_tranche_id: tranche ID to use for the SLRU's per-buffer LWLocks.
* bank_tranche_id: tranche ID to use for the bank LWLocks.
* sync_handler: which set of functions to use to handle sync requests
+ * long_segment_names: use short or long segment names
*/
void
SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
@@ -644,7 +645,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
shared->page_number[slotno] == pageno &&
shared->page_status[slotno] != SLRU_PAGE_READ_IN_PROGRESS)
{
- /* See comments for SlruRecentlyUsed macro */
+ /* See comments for SlruRecentlyUsed() */
SlruRecentlyUsed(shared, slotno);
/* update the stats counter of pages found in the SLRU */
--
2.51.0
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-03 02:22 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 01:47 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
@ 2025-09-05 05:11 ` Michael Paquier <michael@paquier.xyz>
2025-09-05 05:41 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Michael Paquier @ 2025-09-05 05:11 UTC (permalink / raw)
To: Julien Rouhaud <rjuju123@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org
On Fri, Sep 05, 2025 at 09:47:24AM +0800, Julien Rouhaud wrote:
> I caught a couple of other minor outdated things in the comments when finishing
> rebasing my own work:
>
> - long_segment_names has been added to SimpleLruInit
> - SlruRecentlyUsed is not a macro anymore
Right. The source of each mistake is different, both affect v17~.
Applied.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: Update outdated references to SLRU ControlLock
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
2025-09-03 02:22 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 01:47 ` Re: Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 05:11 ` Re: Update outdated references to SLRU ControlLock Michael Paquier <michael@paquier.xyz>
@ 2025-09-05 05:41 ` Julien Rouhaud <rjuju123@gmail.com>
0 siblings, 0 replies; 8+ messages in thread
From: Julien Rouhaud @ 2025-09-05 05:41 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers@lists.postgresql.org
On Fri, Sep 05, 2025 at 02:11:18PM +0900, Michael Paquier wrote:
> On Fri, Sep 05, 2025 at 09:47:24AM +0800, Julien Rouhaud wrote:
> > I caught a couple of other minor outdated things in the comments when finishing
> > rebasing my own work:
> >
> > - long_segment_names has been added to SimpleLruInit
> > - SlruRecentlyUsed is not a macro anymore
>
> Right. The source of each mistake is different, both affect v17~.
> Applied.
Thanks!
^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2025-09-05 05:41 UTC | newest]
Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-09-01 03:32 Update outdated references to SLRU ControlLock Julien Rouhaud <rjuju123@gmail.com>
2025-09-01 05:05 ` Michael Paquier <michael@paquier.xyz>
2025-09-01 05:19 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-03 01:21 ` Michael Paquier <michael@paquier.xyz>
2025-09-03 02:22 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 01:47 ` Julien Rouhaud <rjuju123@gmail.com>
2025-09-05 05:11 ` Michael Paquier <michael@paquier.xyz>
2025-09-05 05:41 ` Julien Rouhaud <rjuju123@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox