On Sat, Mar 25, 2023 at 11:17 AM Andres Freund <andres@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
