On Sat, Mar 25, 2023 at 11:17 AM Andres Freund <and...@anarazel.de> wrote: > Thinking more about this, I think there's no inherent deadlock danger with > reading the VM while holding a buffer lock, "just" an efficiency issue. If we > avoid needing to do IO nearly all the time, by trying to pin the right page > before extending, it's probably good enough.
Uh, it was quite possible for lazy_vacuum_heap_page() to do that up until very recently (it was fixed by my commit 980ae17310). Since it would call visibilitymap_get_status() with an exclusive buffer lock on the heap page, which sometimes had to change the VM page. It potentially did an IO at that point, to read in a later VM page to the caller's initially-pinned one. In other words, up until recently there was a strange idiom used by lazy_vacuum_heap_page/lazy_vacuum_heap_rel, where we'd abuse visibilitymap_get_status() as a replacement for calling visibilitymap_pin() right before acquire a heap page buffer lock. But now the second heap pass does it the same way as the first heap pass. (Even still, I have no reason to believe that the previous approach was all that bad; it was just a bit ugly.) There are still a few visibilitymap_get_status()-with-buffer-lock calls in vacuumlazy.c, FWIW, but they don't run the risk of needing to change the vmbuffer we have pinned with the heap page buffer lock held. -- Peter Geoghegan
