On Wed, Apr 22, 2020 at 8:33 PM Andres Freund <andres@anarazel.de> wrote:
> On 2020-04-22 20:08:42 -0700, Peter Geoghegan wrote:
> > I can get Valgrind to complain about it when the regression tests are
> > run with the attached patch applied.
>
> Nice! Have you checked how much of an incremental slowdown this causes?
No, but I didn't notice much of a slowdown.
> > This patch is very rough -- it was just the first thing that I tried.
> > I don't know how Valgrind remembers the status of shared memory
> > regions across backends when they're marked with
> > VALGRIND_MAKE_MEM_NOACCESS(). Even still, this idea looks promising. I
> > should try to come up with a committable patch before too long.
>
> IIRC valgrind doesn't at all share access markings across processes.
I didn't think so.
> > The good news is that the error I showed is the only error that I see,
> > at least with this rough patch + "make installcheck". It's possible
> > that the patch isn't as effective as it could be, though. For one
> > thing, it definitely won't detect incorrect buffer accesses where a
> > pin is held but a buffer lock is not held. That seems possible, but a
> > bit harder.
>
> Given hint bits it seems fairly hard to make that a reliable check.
I don't follow. It doesn't have to be a perfect check. Detecting if
there is *any* buffer lock held at all would be a big improvement.
> Why aren't we doing this in PinBuffer() and PinBuffer_Locked(), but at
> their callsites?
I wrote this patch in a completely careless manner in less than 10
minutes, just to see how hard it was (I thought that it might have
been much harder). I wasn't expecting you to review it. I thought that
I was clear about that.
--
Peter Geoghegan
