Hi,
On 2021-05-06 13:35:56 -0700, Peter Geoghegan wrote:
> On Thu, May 6, 2021 at 12:32 PM Andres Freund <andres@anarazel.de> wrote:
> > I think it'd be a good idea to audit the other uses of
> > all_visible_according_to_vm to make sure there's no issues there. Can't
> > this e.g. make us miss setting all-visible in
> >
> > /*
> > * Handle setting visibility map bit based on what the VM said about
> > * the page before pruning started, and using prunestate
> > */
> > if (!all_visible_according_to_vm && prunestate.all_visible)
>
> I don't think so, because it's the inverse case -- the condition that
> you quote is concerned with the case where we found the VM all_visible
> bit to not be set earlier, and then found that we could set it now.
Uh, yes, that is exactly my point. Because all_visible_according_to_vm
is "falsely true", we'll not enter this branch, even though we actually
would want to mark it all visible again. If we did update
all_visible_according_to_vm after acquiring the content lock, we would
have entered this branch, no?
Greetings,
Andres Freund