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/
