hio.c does visibilitymap_pin()/IO while holding buffer lock

Hi,

Starting with

commit 7db0cd2145f2bce84cac92402e205e4d2b045bf2
Author: Tomas Vondra <tomas(dot)vondra(at)postgresql(dot)org>
Date: 2021-01-17 22:11:39 +0100

Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZE

RelationGetBufferForTuple does

/*
* The page is empty, pin vmbuffer to set all_frozen bit.
*/
if (options & HEAP_INSERT_FROZEN)
{
Assert(PageGetMaxOffsetNumber(BufferGetPage(buffer)) == 0);
visibilitymap_pin(relation, BufferGetBlockNumber(buffer), vmbuffer);
}

while holding a buffer lock. visibilitymap_pin() reads pages, if vmbuffer
doesn't already point to the right block.

The lock ordering rules are to lock VM pages *before* locking heap pages.

I think the reason this hasn't yet bitten us badly, is that INSERT_FROZEN
effectively requires that the relation is access exclusive locked. There
shouldn't be other backends locking multiple buffers in the relation (bgwriter
/ checkpointer can lock a single buffer at a time, but that's it).

I see roughly two ways forward:

1) We add a comment explaining why it's safe to violate lock ordering rules in
this one situation

2) Change relevant code so that we only return a valid vmbuffer if we could do
so without blocking / IO and, obviously, skip updating the VM if we
couldn't get the buffer.

Greetings,

Andres Freund