Hi, Currently lwlock acquisition, whether direct, or via the LockBuffer() wrapper, have a mode argument. I don't like that much, for three reasons:
1) I've tried to add annotations for static analyzers to help with locking correctness. The ones I looked at don't support annotating shared/exclusive locks where the mode is specified as a variable. 2) When doing performance analysis it's quite useful to be able to see the difference between exlusive and shared acquisition. Typically all one has access to is the symbol name though. 3) I don't like having the unnecessary branches for the lock mode, after all a lot of the lock protected code is fairly hot. It's pretty unnecessary because the caller almost (?) always uses a static lock mode. Therefore I'd like to replace the current lock functions with ones where the lock mode is specified as part of the function name rather than an argument. To avoid unnecessary backward compat pains it seems best to first introduce compat wrappers using the current signature, and then subsequently replace in-core callers with the direct calls. There's several harder calls though: 1) All of the above would benefit from lock release also being annotated with the lock mode. That'd be a lot more invasive however. I think it'd be best to add explicit functions (which would just assert held_lwlocks[] being correct), but keep a wrapper that determines the current lock level using held_lwlocks. 2) For performance it'd be nice if we could move the BufferIsLocal() checks for LockBuffer* into the caller. Even better would be if we made them inline wrappers around LWLockAcquire(Shared|Exclusive). However, as the latter would require making BufferDescriptorGetContentLock() available in bufmgr.h I think that's not worth it. So I think we'd be best off having LockBufferExclusive() be a static inline wrapper doing the BufferIsLocal() check and then calling LockBufferExclusiveImpl which'd do the LWLockAcquireExclusive(). Thoughts? Greetings, Andres Freund
