Fix bank selection logic in SLRU

[Yura Sokolov]

Good day, hackers.

Due to long discussion about SLRU size configuration [1] and code 
evolution, non-serious bug were introduced:

- intermediate versions assumed cache size is always power of 2,
- therefore to determine bank simple binary-and with mask were used:

    bankno = pageno & ctl->bank_mask;

- final merged version allows arbitrary cache size for every cache
  type, but code for bankno were not fixed.

It is not critical bug, since it doesn't hurt correctness just 
performance. In worst case only one bank will be used.

I attach the patch, that changes SlruCtlData->bank_mask to ->nbanks,
and changes calculation to modulo operation.

    bankno = pageno % ctl->nbanks;

Probably, instead of modulo operation could be used multiplication:

    bankno = ((uint64) murmurhash32(pageno) * ctl->nbanks) >> 32;

But I didn't bother to measure does it pay for or not.

[1] 
https://www.postgresql.org/message-id/flat/CAFiTN-vzDvNz%3DExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A%40mail.gmail.com

Regards,
Yura Sokolov aka funny-falconFrom 4b438e67a79d77bf2caf3e5a0386bb7700d329ba Mon Sep 17 00:00:00 2001
From: Yura Sokolov <y.soko...@postgrespro.ru>
Date: Tue, 10 Dec 2024 15:38:02 +0300
Subject: [PATCH v1] Fix SLRU bank selection.

Due to code evolution, nbanks is not power of two any more. But still
binary & were used to obtain bankno.
---
 src/backend/access/transam/slru.c | 6 +++---
 src/include/access/slru.h         | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index e7f73bf4275..afedb5c039f 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -343,7 +343,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
 	ctl->long_segment_names = long_segment_names;
-	ctl->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
+	ctl->nbanks = nbanks;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -606,7 +606,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
 {
 	SlruShared	shared = ctl->shared;
 	LWLock	   *banklock = SimpleLruGetBankLock(ctl, pageno);
-	int			bankno = pageno & ctl->bank_mask;
+	int			bankno = pageno % ctl->nbanks;
 	int			bankstart = bankno * SLRU_BANK_SIZE;
 	int			bankend = bankstart + SLRU_BANK_SIZE;
 
@@ -1177,7 +1177,7 @@ SlruSelectLRUPage(SlruCtl ctl, int64 pageno)
 		int			bestinvalidslot = 0;	/* keep compiler quiet */
 		int			best_invalid_delta = -1;
 		int64		best_invalid_page_number = 0;	/* keep compiler quiet */
-		int			bankno = pageno & ctl->bank_mask;
+		int			bankno = pageno % ctl->nbanks;
 		int			bankstart = bankno * SLRU_BANK_SIZE;
 		int			bankend = bankstart + SLRU_BANK_SIZE;
 
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index 97e612cd100..fabea220290 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -129,9 +129,9 @@ typedef struct SlruCtlData
 	SlruShared	shared;
 
 	/*
-	 * Bitmask to determine bank number from page number.
+	 * Number of banks to determine bank number from page number.
 	 */
-	bits16		bank_mask;
+	bits16		nbanks;
 
 	/*
 	 * If true, use long segment file names.  Otherwise, use short file names.
@@ -179,7 +179,7 @@ SimpleLruGetBankLock(SlruCtl ctl, int64 pageno)
 {
 	int			bankno;
 
-	bankno = pageno & ctl->bank_mask;
+	bankno = pageno % ctl->nbanks;
 	return &(ctl->shared->bank_locks[bankno].lock);
 }
 
-- 
2.43.0