On Wed, Apr 22, 2020 at 8:33 PM Andres Freund <and...@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
