Hi, On 2025-05-19 18:13:02 +0200, Tomas Vondra wrote: > I believe enabling data checksums simply makes it more severe, because > the BufferGetLSNAtomic() has to obtain header lock, which uses the same > "state" field, with exactly the same retry logic. It can probably happen > even without it, but as the lock is exclusive, it also "serializes" the > access, making the conflicts more likely. > > BufferGetLSNAtomic does this: > > bufHdr = GetBufferDescriptor(buffer - 1); > buf_state = LockBufHdr(bufHdr); > lsn = PageGetLSN(page); > UnlockBufHdr(bufHdr, buf_state); > > AFAICS the lock is needed simply to read a consistent value from the > page header, but maybe we could have an atomic variable with a copy of > the LSN in the buffer descriptor?
I think we can do better - something like #ifdef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY lsn = PageGetLSN(page); #else buf_state = LockBufHdr(bufHdr); lsn = PageGetLSN(page); UnlockBufHdr(bufHdr, buf_state); #endif All perf relevant systems support reading 8 bytes without a chance of tearing... Greetings, Andres Freund