On 2024-Feb-02, Dilip Kumar wrote:
> I have checked the patch and it looks fine to me other than the above
> question related to memory barrier usage one more question about the
> same, basically below to instances 1 and 2 look similar but in 1 you
> are not using the memory write_barrier whereas in 2 you are using the
> write_barrier, why is it so? I mean why the reordering can not happen
> in 1 and it may happen in 2?
What I was thinking is that there's a lwlock operation just below, which
acts as a barrier. But I realized something more important: there are
only two places that matter, which are SlruSelectLRUPage and
SimpleLruZeroPage. The others are all initialization code that run at a
point where there's no going to be any concurrency in SLRU access, so we
don't need barriers anyway. In SlruSelectLRUPage we definitely don't
want to evict the page that SimpleLruZeroPage has initialized, starting
from the point where it returns that new page to its caller.
But if you consider the code of those two routines, you realize that the
only time an equality between latest_page_number and "this_page_number"
is going to occur, is when both pages are in the same bank ... and both
routines are required to be holding the bank lock while they run, so in
practice this is never a problem.
We need the atomic write and atomic read so that multiple processes
processing pages in different banks can update latest_page_number
simultaneously. But the equality condition that we're looking for?
it can never happen concurrently.
In other words, these barriers are fully useless.
(We also have SimpleLruTruncate, but I think it's not as critical to
have a barrier there anyhow: accessing a slightly outdated page number
could only be a problem if a bug elsewhere causes us to try to truncate
in the current page. I think we only have this code there because we
did have such bugs in the past, but IIUC this shouldn't happen anymore.)
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/